Skip to content
Permalink
Browse files Browse the repository at this point in the history
rpm: prevent directory traversal when extracting
A malicious tar archive with a member consisting of multiple "parent"
elements could cause a file outside of the target directory to be
overwritten.

Fixes: CVE-2021-3762
Signed-off-by: Hank Donnay <hdonnay@redhat.com>
  • Loading branch information
hdonnay committed Sep 28, 2021
1 parent 94dd31b commit 691f202
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 3 deletions.
14 changes: 11 additions & 3 deletions rpm/packagescanner.go
Expand Up @@ -170,7 +170,7 @@ func (ps *Scanner) Scan(ctx context.Context, layer *claircore.Layer) ([]*clairco
continue
}
// Build the path on the filesystem.
tgt := filepath.Join(root, filepath.Clean(h.Name))
tgt := relPath(root, h.Name)
// Since tar, as a format, doesn't impose ordering requirements, make
// sure to create all parent directories of the current entry.
d := filepath.Dir(tgt)
Expand Down Expand Up @@ -212,12 +212,12 @@ func (ps *Scanner) Scan(ctx context.Context, layer *claircore.Layer) ([]*clairco
stats.Reg++
case tar.TypeSymlink:
// Normalize the link target into the root.
ln := filepath.Join(root, filepath.Clean(h.Linkname))
ln := relPath(root, h.Linkname)
err = os.Symlink(ln, tgt)
stats.Symlink++
case tar.TypeLink:
// Normalize the link target into the root.
ln := filepath.Join(root, filepath.Clean(h.Linkname))
ln := relPath(root, h.Linkname)
_, exists := os.Lstat(ln)
switch {
case errors.Is(exists, nil):
Expand Down Expand Up @@ -469,3 +469,11 @@ func checkMagic(ctx context.Context, r io.Reader) bool {

return false
}

// RelPath takes a member and forcibly interprets it as a path underneath root.
//
// This should be used anytime a path for a new file on disk is needed when
// unpacking a tar.
func relPath(root, member string) string {
return filepath.Join(root, filepath.Join("/", member))
}
37 changes: 37 additions & 0 deletions rpm/packagescanner_test.go
Expand Up @@ -6,6 +6,7 @@ import (
"os"
"os/exec"
"path/filepath"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -1636,3 +1637,39 @@ func TestScan(t *testing.T) {
t.Error(cmp.Diff(got, want))
}
}

func TestRelPath(t *testing.T) {
root := `/tmp/fakedata`
tt := [][2]string{
{
"dev/null",
"/dev/null",
},
{
"dev/null",
"./dev/null",
},
{
"dev/null",
"dev/null",
},
{
"dev/null",
strings.Repeat("../", 10) + "dev/null",
},
{
"dev/null",
strings.Repeat("../", 10) + "dev/./../dev/null",
},
}

for _, tc := range tt {
want := filepath.Join(root, tc[0])
t.Logf("in: %q + %q", root, tc[1])
got := relPath(root, tc[1])
t.Logf("got: %q, want: %q", got, want)
if got != want {
t.Error()
}
}
}

0 comments on commit 691f202

Please sign in to comment.