Skip to content
This repository was archived by the owner on Jun 30, 2023. It is now read-only.

Commit

Permalink
Fix bug where directories inside rewritten jars had incorrect lengths
Browse files Browse the repository at this point in the history
Previously, when patching certain jars the length calculations were slightly off
due to our usage of CreateRaw to create zip headers to represent empty directories.
These slight errors don't seem to actually break the jars from java's perspective,
java still opens them fine. But Finder on MacOS complains that they are invalid
if someone tries to open them as a zip. Inspecting them with `zipinfo -v` shows
a number of warnings along the lines of "There are an extra -2 bytes preceding
this file".

This PR fixes this by using CreateHeader instead of CreateRaw. I also added
tests to ensure that this corruption doesn't happen again.
  • Loading branch information
ddworken committed Jan 5, 2022
1 parent 8d1d5f7 commit 978077f
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 2 deletions.
4 changes: 2 additions & 2 deletions jar/rewrite.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,8 @@ func Rewrite(w io.Writer, zr *zip.Reader) error {

copyFile:
if zipItem.Mode().IsDir() {
// Copy() only works on files.
if _, err := zw.CreateRaw(&zipItem.FileHeader); err != nil {
// Copy() only works on files, so manually create the directory entry
if _, err := zw.CreateHeader(&zipItem.FileHeader); err != nil {
return fmt.Errorf("failed to copy zip directory %s: %v", zipItem.Name, err)
}
} else {
Expand Down
72 changes: 72 additions & 0 deletions jar/rewrite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,11 @@ import (
"fmt"
"io"
"os"
"os/exec"
"path"
"path/filepath"
"regexp"

"testing"

"github.com/google/go-cmp/cmp"
Expand Down Expand Up @@ -337,3 +340,72 @@ func TestAutoMitigateSignedJAR(t *testing.T) {
})
}
}

func TestAutoMitigatedJarsAreCorrectlyFormed(t *testing.T) {
if _, err := exec.LookPath("zipinfo"); err != nil {
t.Skip("zipinfo not available, skipping test")
}

testCases := []string{
"arara.jar",
"shadow-6.1.0.jar",
"arara.signed.jar",
"bad_jar_in_jar_in_jar.jar",
"bad_jar_in_jar.jar",
"good_jar_in_jar_in_jar.jar",
"good_jar_in_jar.jar",
"helloworld.jar",
"helloworld.signed.jar",
"log4j-core-2.12.1.jar",
"log4j-core-2.14.0.jar",
"log4j-core-2.15.0.jar",
"log4j-core-2.16.0.jar",
"log4j-core-2.1.jar",
"safe1.jar",
"safe1.signed.jar",
}
for _, name := range testCases {
t.Run(name, func(t *testing.T) {
// Set up
src := testdataPath(name)
dest := filepath.Join(t.TempDir(), name)

cpFile(t, dest, src)

// Mitigate
if err := autoMitigateJAR(dest); err != nil {
t.Fatalf("autoMitigateJar(%s) failed: %v", dest, err)
}

// Check that the jars were actually mitigated
before, err := zip.OpenReader(src)
if err != nil {
t.Fatalf("zip.OpenReader(%q) failed: %v", src, err)
}
defer before.Close()
after, err := zip.OpenReader(dest)
if err != nil {
t.Fatalf("zip.OpenReader(%q) failed: %v", dest, err)
}
defer after.Close()
checkJARs(t, func(name string) bool {
return name == "JndiLookup.class" ||
name == "SERVER.SF" ||
name == "SERVER.RSA"
}, &before.Reader, &after.Reader)

// Check that they are well formed
out, err := exec.Command("zipinfo", "-v", dest).Output()
if err != nil {
t.Fatalf("zipinfo command failed for dest %s: %v", dest, err)
}
match, err := regexp.MatchString(`There are an extra -\d+ bytes preceding this file`, string(out))
if err != nil {
t.Fatalf("regex failed: %v", err)
}
if match {
t.Fatalf("mitigated jar %s is malformed:\n%v", dest, string(out))
}
})
}
}
Binary file added jar/testdata/shadow-6.1.0.jar
Binary file not shown.

0 comments on commit 978077f

Please sign in to comment.