fix: bound debian ParseControlFile to a single control stanza (#38044)
**Packages-index stanza injection via Debian control file** A `.deb` whose `control` file appends extra paragraphs after a blank line was still accepted, and `ParseControlFile` stored the whole multi-stanza blob in `p.Control`. That blob is re-emitted verbatim into the generated `Packages` index, so the embedded blank line splits it into separate stanzas and an uploader can smuggle a package entry with an attacker-chosen `Filename` into the shared index. A binary control file only holds one stanza, so parsing now stops at the blank line that terminates it; well-formed packages are unaffected and the new subtest covers the trailing-stanza case. --------- Signed-off-by: wxiaoguang <wxiaoguang@gmail.com> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
co-authored by
GitHub
wxiaoguang
parent
7b4a1a1a11
commit
7134c1f845
@@ -146,15 +146,26 @@ func ParseControlFile(r io.Reader) (*Package, error) {
|
||||
var depends strings.Builder
|
||||
var control strings.Builder
|
||||
|
||||
s := bufio.NewScanner(io.TeeReader(r, &control))
|
||||
// https://www.debian.org/doc/debian-policy/ch-controlfields.html#syntax-of-control-files
|
||||
s := bufio.NewScanner(r)
|
||||
for s.Scan() {
|
||||
line := s.Text()
|
||||
|
||||
trimmed := strings.TrimSpace(line)
|
||||
if trimmed == "" {
|
||||
continue
|
||||
// A binary package control file holds exactly one stanza. Stop at the
|
||||
// blank line that terminates it, otherwise a crafted control file could
|
||||
// smuggle additional stanzas (with attacker-chosen Filename/Package
|
||||
// fields) into the generated repository "Packages" index.
|
||||
if control.Len() == 0 {
|
||||
continue
|
||||
}
|
||||
break
|
||||
}
|
||||
|
||||
control.WriteString(line)
|
||||
control.WriteByte('\n')
|
||||
|
||||
if line[0] == ' ' || line[0] == '\t' {
|
||||
switch key {
|
||||
case "Description":
|
||||
|
||||
@@ -184,4 +184,19 @@ func TestParseControlFile(t *testing.T) {
|
||||
assert.NotNil(t, p)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("SingleStanzaOnly", func(t *testing.T) {
|
||||
// A control file with a trailing stanza must not leak the extra fields into
|
||||
// p.Control, otherwise buildPackagesIndices would emit a second package entry
|
||||
// with an attacker-chosen Filename into the repository "Packages" index.
|
||||
content := bytes.NewBufferString("Package: realpkg\nVersion: 1.0.0\nArchitecture: amd64\nMaintainer: a <a@b.c>\nDescription: real\n\nPackage: openssl\nVersion: 99.0\nArchitecture: amd64\nFilename: pool/main/o/openssl/evil.deb\nDescription: spoofed\n")
|
||||
|
||||
p, err := ParseControlFile(content)
|
||||
assert.NoError(t, err)
|
||||
assert.NotNil(t, p)
|
||||
assert.Equal(t, "realpkg", p.Name)
|
||||
assert.Equal(t, "1.0.0", p.Version)
|
||||
assert.NotContains(t, p.Control, "openssl")
|
||||
assert.NotContains(t, p.Control, "evil.deb")
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user