diff --git a/lib/signjar/digest.go b/lib/signjar/digest.go index b8b97a7..02bc2fe 100644 --- a/lib/signjar/digest.go +++ b/lib/signjar/digest.go @@ -99,7 +99,7 @@ func updateManifest(jar *zipslicer.Directory, hash crypto.Hash) (*JarDigest, err } else if jd.Manifest == nil { return nil, errors.New("JAR did not contain a manifest") } - files, err := ParseManifest(jd.Manifest) + files, malformed, err := parseManifest(jd.Manifest) if err != nil { return nil, err } @@ -143,7 +143,7 @@ func updateManifest(jar *zipslicer.Directory, hash crypto.Hash) (*JarDigest, err changed = true } } - if changed { + if changed || malformed { jd.Manifest = files.Dump() } return jd, nil diff --git a/lib/signjar/manifest.go b/lib/signjar/manifest.go index 8b7a8c9..ae7a139 100644 --- a/lib/signjar/manifest.go +++ b/lib/signjar/manifest.go @@ -37,6 +37,8 @@ const ( manifestName = metaInf + "MANIFEST.MF" ) +var ErrManifestLineEndings = errors.New("manifest has incorrect line ending sequence") + type FilesMap struct { Main http.Header Order []string @@ -44,12 +46,19 @@ type FilesMap struct { } func ParseManifest(manifest []byte) (files *FilesMap, err error) { - sections, err := splitManifest(manifest) + files, malformed, err := parseManifest(manifest) if err != nil { return nil, err + } else if malformed { + return nil, ErrManifestLineEndings } + return files, nil +} + +func parseManifest(manifest []byte) (files *FilesMap, malformed bool, err error) { + sections, malformed := splitManifest(manifest) if len(sections) == 0 { - return nil, errors.New("manifest has no sections") + return nil, false, errors.New("manifest has no sections") } files = &FilesMap{ Order: make([]string, 0, len(sections)-1), @@ -61,20 +70,20 @@ func ParseManifest(manifest []byte) (files *FilesMap, err error) { } hdr, err := parseSection(section) if err != nil { - return nil, err + return nil, false, err } if i == 0 { files.Main = hdr } else { name := hdr.Get("Name") if name == "" { - return nil, errors.New("manifest has section with no \"Name\" attribute") + return nil, false, errors.New("manifest has section with no \"Name\" attribute") } files.Order = append(files.Order, name) files.Files[name] = hdr } } - return files, nil + return files, malformed, nil } func (m *FilesMap) Dump() []byte { @@ -89,7 +98,8 @@ func (m *FilesMap) Dump() []byte { return out.Bytes() } -func splitManifest(manifest []byte) ([][]byte, error) { +func splitManifest(manifest []byte) ([][]byte, bool) { + var malformed bool sections := make([][]byte, 0) for len(manifest) != 0 { i1 := bytes.Index(manifest, []byte("\r\n\r\n")) @@ -100,20 +110,23 @@ func splitManifest(manifest []byte) ([][]byte, error) { idx = i1 + 4 case i2 >= 0: idx = i2 + 2 - case len(sections) == 0: - // as a special case, accept a single final newline if it's the only section - if manifest[len(manifest)-1] == '\n' { - return [][]byte{manifest}, nil - } - fallthrough default: - return nil, errors.New("trailing bytes after last newline") + // If there is not a proper 2x line ending, + // then it's technically not valid but we can sign it anyway + // as long as it gets rewritten with correct endings. + idx = len(manifest) + malformed = true } section := manifest[:idx] manifest = manifest[idx:] + if len(bytes.TrimSpace(section)) == 0 { + // Excessive line endings have created an empty section + malformed = true + continue + } sections = append(sections, section) } - return sections, nil + return sections, malformed } func parseSection(section []byte) (http.Header, error) { @@ -145,9 +158,9 @@ func hashSection(hash crypto.Hash, section []byte) string { // Transform a MANIFEST.MF into a *.SF by digesting each section with the // specified hash func DigestManifest(manifest []byte, hash crypto.Hash, sectionsOnly, apkV2 bool) ([]byte, error) { - sections, err := splitManifest(manifest) - if err != nil { - return nil, err + sections, malformed := splitManifest(manifest) + if malformed { + return nil, ErrManifestLineEndings } hashName := x509tools.HashNames[hash] if hashName == "" { diff --git a/lib/signjar/manifest_test.go b/lib/signjar/manifest_test.go index b4e8192..418f229 100644 --- a/lib/signjar/manifest_test.go +++ b/lib/signjar/manifest_test.go @@ -50,7 +50,7 @@ Eggs: bacon require.NoError(t, err) assert.Equal(t, expected, parsed) }) - t.Run("Truncated", func(t *testing.T) { + t.Run("TruncatedMain", func(t *testing.T) { const manifest = "Manifest-Version: 1.0\n" expected := &FilesMap{ Main: http.Header{ @@ -59,14 +59,50 @@ Eggs: bacon Order: []string{}, Files: map[string]http.Header{}, } - parsed, err := ParseManifest([]byte(manifest)) + _, err := ParseManifest([]byte(manifest)) + require.ErrorIs(t, err, ErrManifestLineEndings) + parsed, malformed, err := parseManifest([]byte(manifest)) require.NoError(t, err) + assert.True(t, malformed) assert.Equal(t, expected, parsed) }) - t.Run("InvalidTruncated", func(t *testing.T) { + t.Run("TruncatedFile", func(t *testing.T) { const manifest = "Manifest-Version: 1.0\n\nName: foo\n" + file := http.Header{ + "Name": []string{"foo"}, + } + expected := &FilesMap{ + Main: http.Header{ + "Manifest-Version": []string{"1.0"}, + }, + Order: []string{"foo"}, + Files: map[string]http.Header{"foo": file}, + } _, err := ParseManifest([]byte(manifest)) - require.Error(t, err) + require.ErrorIs(t, err, ErrManifestLineEndings) + parsed, malformed, err := parseManifest([]byte(manifest)) + require.NoError(t, err) + assert.True(t, malformed) + assert.Equal(t, expected, parsed) + }) + t.Run("TrailingWhitespace", func(t *testing.T) { + const manifest = "Manifest-Version: 1.0\n\nName: foo\n\n\n" + file := http.Header{ + "Name": []string{"foo"}, + } + expected := &FilesMap{ + Main: http.Header{ + "Manifest-Version": []string{"1.0"}, + }, + Order: []string{"foo"}, + Files: map[string]http.Header{"foo": file}, + } + _, err := ParseManifest([]byte(manifest)) + require.ErrorIs(t, err, ErrManifestLineEndings) + parsed, malformed, err := parseManifest([]byte(manifest)) + require.NoError(t, err) + assert.True(t, malformed) + assert.Equal(t, expected, parsed) }) t.Run("InvalidNoName", func(t *testing.T) { const manifest = "Manifest-Version: 1.0\n\nFoo: bar\n\n" diff --git a/lib/signjar/verify.go b/lib/signjar/verify.go index 8628a69..34843d3 100644 --- a/lib/signjar/verify.go +++ b/lib/signjar/verify.go @@ -211,9 +211,9 @@ func verifySigFile(sigfile, manifest []byte) (http.Header, error) { // if the whole-file digest passed then skip the sections return sfParsed.Main, nil } - sections, err := splitManifest(manifest) - if err != nil { - return nil, err + sections, malformed := splitManifest(manifest) + if malformed { + return nil, ErrManifestLineEndings } sectionMap := make(map[string][]byte, len(sections)-1) for i, section := range sections {