Skip to content

Commit

Permalink
fix(jar): be even more tolerant of missing line endings
Browse files Browse the repository at this point in the history
  • Loading branch information
mtharp committed Aug 9, 2023
1 parent 70e11a7 commit 00db40d
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 26 deletions.
4 changes: 2 additions & 2 deletions lib/signjar/digest.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down
47 changes: 30 additions & 17 deletions lib/signjar/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,28 @@ const (
manifestName = metaInf + "MANIFEST.MF"
)

var ErrManifestLineEndings = errors.New("manifest has incorrect line ending sequence")

type FilesMap struct {
Main http.Header
Order []string
Files map[string]http.Header
}

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),
Expand All @@ -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 {
Expand All @@ -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"))
Expand All @@ -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) {
Expand Down Expand Up @@ -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 == "" {
Expand Down
44 changes: 40 additions & 4 deletions lib/signjar/manifest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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"
Expand Down
6 changes: 3 additions & 3 deletions lib/signjar/verify.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 00db40d

Please sign in to comment.