Skip to content

Commit 57549bf

Browse files
rolandshoemakerthatnealpatel
authored andcommitted
sumdb: ignore unrelated hashes in Lookup
During Lookup, if the server returns a validly signed record for an unrelated module which contains in the signed tree head another hash for the module which was requested in the "extension" portion of the signed tree head (anything following the tree hash, but before the signatures), _only_ the unrelated hashes are validated by checking the relevant tiles, ignoring the portion in the tree head. Lookup would then look for hash lines which match the requested module path in the fully returned record, not just the validated portion, causing only the hash appended to the tree head portion to be returned. This would allow a malicious sumdb instance to return a validly signed response for a module from which Lookup would return a hash which is not actually present in the tree. This would allow a conspiring malicious sumdb and module proxy (or a module proxy which is proxying the sumdb _and has the sumdb key_) to serve malicious module content which is not recorded in the transparency log. The fix is quite simple, instead of caching the full record, we only cache the hash lines returned by tlog.ParseRecord, which are validated. Lookup will then only extract matching hashes from the validated portion, instead of the full record. Thanks to Mundur (https://github.com/M0nd0R) for reporting this issue. Fixes CVE-2026-56864 Fixes golang/go#80745 Change-Id: I4a418620e8e4e8ab5f7ec41b0ed443a92e9be6e1 Reviewed-on: https://go-review.googlesource.com/c/mod/+/815000 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Auto-Submit: Gopher Robot <gobot@golang.org> Reviewed-by: Neal Patel <nealpatel@google.com> LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
1 parent 96f62ae commit 57549bf

2 files changed

Lines changed: 50 additions & 1 deletion

File tree

sumdb/client.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ func (c *Client) Lookup(path, vers string) (lines []string, err error) {
274274
c.ops.WriteCache(file, data)
275275
}
276276

277-
return cached{data, nil}
277+
return cached{text, nil}
278278
}).(cached)
279279
if result.err != nil {
280280
return nil, result.err

sumdb/client_test.go

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,55 @@ func TestClientGONOSUMDB(t *testing.T) {
189189
}
190190
}
191191

192+
func TestRejectUnauthenticatedLines(t *testing.T) {
193+
tc := newTestClient(t)
194+
195+
data := "golang.org/x/good v1.0.0 h1:7uVkIFmeBqHfdjD+gZwtXXI+RODJ2Wc4O7MPEh/QiW4=\n"
196+
id := tc.treeSize
197+
tc.treeSize++
198+
rec, err := tlog.FormatRecord(id, []byte(data))
199+
if err != nil {
200+
t.Fatal(err)
201+
}
202+
hashes, err := tlog.StoredHashesForRecordHash(id, tlog.RecordHash([]byte(data)), tc)
203+
if err != nil {
204+
t.Fatal(err)
205+
}
206+
tc.hashes = append(tc.hashes, hashes...)
207+
208+
// Create lookup result.
209+
h, err := tlog.TreeHash(tc.treeSize, tc)
210+
if err != nil {
211+
t.Fatal(err)
212+
}
213+
text := append(tlog.FormatTree(tlog.Tree{N: tc.treeSize, Hash: h}), []byte("golang.org/x/bad v1.0.0 h1:7uVkIFmeBqHfdjD+gZwtXXI+RODJ2Wc4O7MPEh/QiW4=\n")...)
214+
signed, err := note.Sign(&note.Note{Text: string(text)}, tc.signer)
215+
if err != nil {
216+
t.Fatal(err)
217+
}
218+
219+
lookupRes := append(rec, signed...)
220+
tc.remote["/lookup/golang.org/x/bad@v1.0.0"] = lookupRes
221+
222+
// Create new tiles.
223+
tiles := tlog.NewTiles(tc.tileHeight, id, tc.treeSize)
224+
for _, tile := range tiles {
225+
data, err := tlog.ReadTileData(tile, tc)
226+
if err != nil {
227+
t.Fatal(err)
228+
}
229+
tc.remote["/"+tile.Path()] = data
230+
}
231+
232+
lines, err := tc.client.Lookup("golang.org/x/bad", "v1.0.0")
233+
if err != nil {
234+
t.Fatal(err)
235+
}
236+
if len(lines) != 0 {
237+
t.Errorf("Lookup(%q): expected no hashes, got %q", "golang.org/x/bad", strings.Join(lines, "\n"))
238+
}
239+
}
240+
192241
// A testClient is a self-contained client-side testing environment.
193242
type testClient struct {
194243
t *testing.T // active test

0 commit comments

Comments
 (0)