2021-08-23 15:14:53 +02:00
|
|
|
//go:build !windows
|
2019-05-30 11:15:09 -07:00
|
|
|
// +build !windows
|
|
|
|
|
|
|
|
package chrootarchive
|
|
|
|
|
|
|
|
import (
|
2019-05-30 14:55:52 -07:00
|
|
|
gotar "archive/tar"
|
2019-05-30 11:15:09 -07:00
|
|
|
"bytes"
|
|
|
|
"io"
|
|
|
|
"os"
|
2019-05-30 14:55:52 -07:00
|
|
|
"path"
|
2019-05-30 11:15:09 -07:00
|
|
|
"path/filepath"
|
2019-05-30 14:55:52 -07:00
|
|
|
"strings"
|
2019-05-30 11:15:09 -07:00
|
|
|
"testing"
|
|
|
|
|
|
|
|
"github.com/docker/docker/pkg/archive"
|
|
|
|
"golang.org/x/sys/unix"
|
2020-02-07 14:39:24 +01:00
|
|
|
"gotest.tools/v3/assert"
|
2020-11-26 22:39:55 +07:00
|
|
|
"gotest.tools/v3/skip"
|
2019-05-30 11:15:09 -07:00
|
|
|
)
|
|
|
|
|
|
|
|
// Test for CVE-2018-15664
|
|
|
|
// Assures that in the case where an "attacker" controlled path is a symlink to
|
|
|
|
// some path outside of a container's rootfs that we do not copy data to a
|
|
|
|
// container path that will actually overwrite data on the host
|
|
|
|
func TestUntarWithMaliciousSymlinks(t *testing.T) {
|
2020-11-26 22:39:55 +07:00
|
|
|
skip.If(t, os.Getuid() != 0, "skipping test that requires root")
|
2022-10-16 12:59:14 +02:00
|
|
|
dir := t.TempDir()
|
2019-05-30 11:15:09 -07:00
|
|
|
|
|
|
|
root := filepath.Join(dir, "root")
|
|
|
|
|
pkg/chrootarchive: replace system.MkdirAll for os.Mkdir
system.MkdirAll is a special version of os.Mkdir to handle creating directories
using Windows volume paths ("\\?\Volume{4c1b02c1-d990-11dc-99ae-806e6f6e6963}").
This may be important when MkdirAll is used, which traverses all parent paths to
create them if missing (ultimately landing on the "volume" path).
Commit 62f648b06137a7e21b800f63ac403f7bb4d4f5b4 introduced the system.MkdirAll
calls, as a change was made in applyLayer() for Windows to use Windows volume
paths as an alternative for chroot (which is not supported on Windows). Later
iteractions changed this to regular Windows long-paths (`\\?\<path>`) in
230cfc6ed21e9398b9b3df765e6c02e90031d728, and 9b648dfac6453de5944ee4bb749115d85a253a05.
Such paths are handled by the `os` package.
However, in these tests, the parent path already exists (all paths created are
a direct subdirectory within `tmpDir`). It looks like `MkdirAll` here is used
out of convenience to not have to handle `os.ErrExist` errors. As all these
tests are running in a fresh temporary directory, there should be no need to
handle those, and it's actually desirable to produce an error in that case, as
the directory already existing would be unexpected.
Because of the above, this test changes `system.MkdirAll` to `os.Mkdir`. As we
are changing these lines, this patch also changes the legacy octal notation
(`0700`) to the now preferred `0o700`.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-16 13:53:58 +02:00
|
|
|
err := os.Mkdir(root, 0o755)
|
2019-05-30 11:15:09 -07:00
|
|
|
assert.NilError(t, err)
|
|
|
|
|
|
|
|
// Add a file into a directory above root
|
|
|
|
// Ensure that we can't access this file while tarring.
|
2022-10-16 14:09:06 +02:00
|
|
|
err = os.WriteFile(filepath.Join(dir, "host-file"), []byte("I am a host file"), 0o644)
|
2019-05-30 11:15:09 -07:00
|
|
|
assert.NilError(t, err)
|
|
|
|
|
|
|
|
// Create some data which which will be copied into the "container" root into
|
|
|
|
// the symlinked path.
|
|
|
|
// Before this change, the copy would overwrite the "host" content.
|
|
|
|
// With this change it should not.
|
|
|
|
data := filepath.Join(dir, "data")
|
pkg/chrootarchive: replace system.MkdirAll for os.Mkdir
system.MkdirAll is a special version of os.Mkdir to handle creating directories
using Windows volume paths ("\\?\Volume{4c1b02c1-d990-11dc-99ae-806e6f6e6963}").
This may be important when MkdirAll is used, which traverses all parent paths to
create them if missing (ultimately landing on the "volume" path).
Commit 62f648b06137a7e21b800f63ac403f7bb4d4f5b4 introduced the system.MkdirAll
calls, as a change was made in applyLayer() for Windows to use Windows volume
paths as an alternative for chroot (which is not supported on Windows). Later
iteractions changed this to regular Windows long-paths (`\\?\<path>`) in
230cfc6ed21e9398b9b3df765e6c02e90031d728, and 9b648dfac6453de5944ee4bb749115d85a253a05.
Such paths are handled by the `os` package.
However, in these tests, the parent path already exists (all paths created are
a direct subdirectory within `tmpDir`). It looks like `MkdirAll` here is used
out of convenience to not have to handle `os.ErrExist` errors. As all these
tests are running in a fresh temporary directory, there should be no need to
handle those, and it's actually desirable to produce an error in that case, as
the directory already existing would be unexpected.
Because of the above, this test changes `system.MkdirAll` to `os.Mkdir`. As we
are changing these lines, this patch also changes the legacy octal notation
(`0700`) to the now preferred `0o700`.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-16 13:53:58 +02:00
|
|
|
err = os.Mkdir(data, 0o755)
|
2019-05-30 11:15:09 -07:00
|
|
|
assert.NilError(t, err)
|
2022-10-16 14:09:06 +02:00
|
|
|
err = os.WriteFile(filepath.Join(data, "local-file"), []byte("pwn3d"), 0o644)
|
2019-05-30 11:15:09 -07:00
|
|
|
assert.NilError(t, err)
|
|
|
|
|
|
|
|
safe := filepath.Join(root, "safe")
|
|
|
|
err = unix.Symlink(dir, safe)
|
|
|
|
assert.NilError(t, err)
|
|
|
|
|
|
|
|
rdr, err := archive.TarWithOptions(data, &archive.TarOptions{IncludeFiles: []string{"local-file"}, RebaseNames: map[string]string{"local-file": "host-file"}})
|
|
|
|
assert.NilError(t, err)
|
|
|
|
|
|
|
|
// Use tee to test both the good case and the bad case w/o recreating the archive
|
|
|
|
bufRdr := bytes.NewBuffer(nil)
|
|
|
|
tee := io.TeeReader(rdr, bufRdr)
|
|
|
|
|
|
|
|
err = UntarWithRoot(tee, safe, nil, root)
|
|
|
|
assert.Assert(t, err != nil)
|
|
|
|
assert.ErrorContains(t, err, "open /safe/host-file: no such file or directory")
|
|
|
|
|
|
|
|
// Make sure the "host" file is still in tact
|
|
|
|
// Before the fix the host file would be overwritten
|
2021-08-24 18:10:50 +08:00
|
|
|
hostData, err := os.ReadFile(filepath.Join(dir, "host-file"))
|
2019-05-30 11:15:09 -07:00
|
|
|
assert.NilError(t, err)
|
|
|
|
assert.Equal(t, string(hostData), "I am a host file")
|
|
|
|
|
|
|
|
// Now test by chrooting to an attacker controlled path
|
|
|
|
// This should succeed as is and overwrite a "host" file
|
|
|
|
// Note that this would be a mis-use of this function.
|
|
|
|
err = UntarWithRoot(bufRdr, safe, nil, safe)
|
|
|
|
assert.NilError(t, err)
|
|
|
|
|
2021-08-24 18:10:50 +08:00
|
|
|
hostData, err = os.ReadFile(filepath.Join(dir, "host-file"))
|
2019-05-30 11:15:09 -07:00
|
|
|
assert.NilError(t, err)
|
|
|
|
assert.Equal(t, string(hostData), "pwn3d")
|
|
|
|
}
|
2019-05-30 14:55:52 -07:00
|
|
|
|
|
|
|
// Test for CVE-2018-15664
|
|
|
|
// Assures that in the case where an "attacker" controlled path is a symlink to
|
|
|
|
// some path outside of a container's rootfs that we do not unwittingly leak
|
|
|
|
// host data into the archive.
|
|
|
|
func TestTarWithMaliciousSymlinks(t *testing.T) {
|
2020-11-26 22:39:55 +07:00
|
|
|
skip.If(t, os.Getuid() != 0, "skipping test that requires root")
|
2021-08-24 18:10:50 +08:00
|
|
|
dir, err := os.MkdirTemp("", t.Name())
|
2019-05-30 14:55:52 -07:00
|
|
|
assert.NilError(t, err)
|
|
|
|
// defer os.RemoveAll(dir)
|
|
|
|
t.Log(dir)
|
|
|
|
|
|
|
|
root := filepath.Join(dir, "root")
|
|
|
|
|
pkg/chrootarchive: replace system.MkdirAll for os.Mkdir
system.MkdirAll is a special version of os.Mkdir to handle creating directories
using Windows volume paths ("\\?\Volume{4c1b02c1-d990-11dc-99ae-806e6f6e6963}").
This may be important when MkdirAll is used, which traverses all parent paths to
create them if missing (ultimately landing on the "volume" path).
Commit 62f648b06137a7e21b800f63ac403f7bb4d4f5b4 introduced the system.MkdirAll
calls, as a change was made in applyLayer() for Windows to use Windows volume
paths as an alternative for chroot (which is not supported on Windows). Later
iteractions changed this to regular Windows long-paths (`\\?\<path>`) in
230cfc6ed21e9398b9b3df765e6c02e90031d728, and 9b648dfac6453de5944ee4bb749115d85a253a05.
Such paths are handled by the `os` package.
However, in these tests, the parent path already exists (all paths created are
a direct subdirectory within `tmpDir`). It looks like `MkdirAll` here is used
out of convenience to not have to handle `os.ErrExist` errors. As all these
tests are running in a fresh temporary directory, there should be no need to
handle those, and it's actually desirable to produce an error in that case, as
the directory already existing would be unexpected.
Because of the above, this test changes `system.MkdirAll` to `os.Mkdir`. As we
are changing these lines, this patch also changes the legacy octal notation
(`0700`) to the now preferred `0o700`.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-16 13:53:58 +02:00
|
|
|
err = os.Mkdir(root, 0o755)
|
2019-05-30 14:55:52 -07:00
|
|
|
assert.NilError(t, err)
|
|
|
|
|
|
|
|
hostFileData := []byte("I am a host file")
|
|
|
|
|
|
|
|
// Add a file into a directory above root
|
|
|
|
// Ensure that we can't access this file while tarring.
|
2022-10-16 14:09:06 +02:00
|
|
|
err = os.WriteFile(filepath.Join(dir, "host-file"), hostFileData, 0o644)
|
2019-05-30 14:55:52 -07:00
|
|
|
assert.NilError(t, err)
|
|
|
|
|
|
|
|
safe := filepath.Join(root, "safe")
|
|
|
|
err = unix.Symlink(dir, safe)
|
|
|
|
assert.NilError(t, err)
|
|
|
|
|
|
|
|
data := filepath.Join(dir, "data")
|
pkg/chrootarchive: replace system.MkdirAll for os.Mkdir
system.MkdirAll is a special version of os.Mkdir to handle creating directories
using Windows volume paths ("\\?\Volume{4c1b02c1-d990-11dc-99ae-806e6f6e6963}").
This may be important when MkdirAll is used, which traverses all parent paths to
create them if missing (ultimately landing on the "volume" path).
Commit 62f648b06137a7e21b800f63ac403f7bb4d4f5b4 introduced the system.MkdirAll
calls, as a change was made in applyLayer() for Windows to use Windows volume
paths as an alternative for chroot (which is not supported on Windows). Later
iteractions changed this to regular Windows long-paths (`\\?\<path>`) in
230cfc6ed21e9398b9b3df765e6c02e90031d728, and 9b648dfac6453de5944ee4bb749115d85a253a05.
Such paths are handled by the `os` package.
However, in these tests, the parent path already exists (all paths created are
a direct subdirectory within `tmpDir`). It looks like `MkdirAll` here is used
out of convenience to not have to handle `os.ErrExist` errors. As all these
tests are running in a fresh temporary directory, there should be no need to
handle those, and it's actually desirable to produce an error in that case, as
the directory already existing would be unexpected.
Because of the above, this test changes `system.MkdirAll` to `os.Mkdir`. As we
are changing these lines, this patch also changes the legacy octal notation
(`0700`) to the now preferred `0o700`.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-16 13:53:58 +02:00
|
|
|
err = os.Mkdir(data, 0o755)
|
2019-05-30 14:55:52 -07:00
|
|
|
assert.NilError(t, err)
|
|
|
|
|
|
|
|
type testCase struct {
|
|
|
|
p string
|
|
|
|
includes []string
|
|
|
|
}
|
|
|
|
|
|
|
|
cases := []testCase{
|
|
|
|
{p: safe, includes: []string{"host-file"}},
|
|
|
|
{p: safe + "/", includes: []string{"host-file"}},
|
|
|
|
{p: safe, includes: nil},
|
|
|
|
{p: safe + "/", includes: nil},
|
|
|
|
{p: root, includes: []string{"safe/host-file"}},
|
|
|
|
{p: root, includes: []string{"/safe/host-file"}},
|
|
|
|
{p: root, includes: nil},
|
|
|
|
}
|
|
|
|
|
|
|
|
maxBytes := len(hostFileData)
|
|
|
|
|
|
|
|
for _, tc := range cases {
|
|
|
|
t.Run(path.Join(tc.p+"_"+strings.Join(tc.includes, "_")), func(t *testing.T) {
|
|
|
|
// Here if we use archive.TarWithOptions directly or change the "root" parameter
|
|
|
|
// to be the same as "safe", data from the host will be leaked into the archive
|
|
|
|
var opts *archive.TarOptions
|
|
|
|
if tc.includes != nil {
|
|
|
|
opts = &archive.TarOptions{
|
|
|
|
IncludeFiles: tc.includes,
|
|
|
|
}
|
|
|
|
}
|
|
|
|
rdr, err := Tar(tc.p, opts, root)
|
|
|
|
assert.NilError(t, err)
|
|
|
|
defer rdr.Close()
|
|
|
|
|
|
|
|
tr := gotar.NewReader(rdr)
|
|
|
|
assert.Assert(t, !isDataInTar(t, tr, hostFileData, int64(maxBytes)), "host data leaked to archive")
|
|
|
|
})
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
func isDataInTar(t *testing.T, tr *gotar.Reader, compare []byte, maxBytes int64) bool {
|
|
|
|
for {
|
|
|
|
h, err := tr.Next()
|
|
|
|
if err == io.EOF {
|
|
|
|
break
|
|
|
|
}
|
|
|
|
assert.NilError(t, err)
|
|
|
|
|
|
|
|
if h.Size == 0 {
|
|
|
|
continue
|
|
|
|
}
|
|
|
|
assert.Assert(t, h.Size <= maxBytes, "%s: file size exceeds max expected size %d: %d", h.Name, maxBytes, h.Size)
|
|
|
|
|
|
|
|
data := make([]byte, int(h.Size))
|
|
|
|
_, err = io.ReadFull(tr, data)
|
|
|
|
assert.NilError(t, err)
|
|
|
|
if bytes.Contains(data, compare) {
|
|
|
|
return true
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
return false
|
|
|
|
}
|