Merge pull request #6960 from tonistiigi/v0.31.2-picks

[v0.31] picks for v0.31.2
This commit is contained in:
CrazyMax
2026-07-16 15:59:13 +02:00
committed by GitHub
25 changed files with 822 additions and 48 deletions

View File

@@ -198,8 +198,6 @@ jobs:
- build
env:
GOOS: freebsd
# https://github.com/hashicorp/vagrant/issues/13652
VAGRANT_DISABLE_STRICT_DEPENDENCY_ENFORCEMENT: 1
steps:
-
name: Checkout

View File

@@ -3,12 +3,10 @@ package common
import (
"context"
"os"
"time"
"github.com/moby/buildkit/util/appcontext"
"github.com/moby/buildkit/util/tracing/delegated"
"github.com/moby/buildkit/util/tracing/detect"
"github.com/pkg/errors"
"github.com/urfave/cli/v3"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/codes"
@@ -16,8 +14,6 @@ import (
"go.opentelemetry.io/otel/trace"
)
const exportTimeout = 50 * time.Millisecond
func AttachAppContext(app *cli.Command) error {
baseCtx := appcontext.Context()
@@ -79,13 +75,7 @@ func AttachAppContext(app *cli.Command) error {
span.End()
}
// Set a rather aggressive timeout for shutting down the tracer provider
// to ensure we don't stall on a non-responsive tracing endpoint for too long
// on shutdown.
ctx, cancel := context.WithTimeoutCause(appcontext.Shutdown(), exportTimeout, errors.WithStack(context.DeadlineExceeded))
defer cancel()
return tp.Shutdown(ctx)
return tp.Shutdown(context.TODO())
}
return nil
}

View File

@@ -18,6 +18,7 @@ import (
"github.com/moby/sys/user"
specs "github.com/opencontainers/runtime-spec/specs-go"
"github.com/pkg/errors"
"golang.org/x/sys/windows"
)
const (
@@ -111,8 +112,129 @@ func sub(m mount.Mount, subPath string) (mount.Mount, func() error, error) {
if err != nil {
return mount.Mount{}, nil, err
}
m.Source = src
return m, func() error { return nil }, nil
realSrc, release, err := resolveAndPinPath(src)
if err != nil {
return mount.Mount{}, nil, errors.Wrapf(err, "resolving and pinning mount source subpath %q", subPath)
}
if err := verifySubpathWithinRoot(m.Source, realSrc, subPath); err != nil {
_ = release()
return mount.Mount{}, nil, err
}
// Use the path of the object represented by the pinned handle. HCS can then
// realize the mount without traversing attacker-controlled reparse points in
// the original source path again.
m.Source = mountSourcePath(realSrc)
return m, release, nil
}
// resolveAndPinPath opens src while following reparse points and returns the
// normalized path of the object represented by that handle. GENERIC_READ makes
// the handle participate in share-access checks without requiring delete access
// to the source. Omitting delete sharing prevents the resolved directory from
// being renamed or deleted before HCS realizes the mount. Write sharing is still
// allowed so a writable cache mount can modify contents while the handle is alive.
func resolveAndPinPath(src string) (string, func() error, error) {
pathPtr, err := windows.UTF16PtrFromString(src)
if err != nil {
return "", nil, err
}
h, err := windows.CreateFile(
pathPtr,
windows.GENERIC_READ,
windows.FILE_SHARE_READ|windows.FILE_SHARE_WRITE,
nil,
windows.OPEN_EXISTING,
windows.FILE_FLAG_BACKUP_SEMANTICS,
0,
)
if err != nil {
return "", nil, err
}
realSrc, err := finalPathNameByHandle(h)
if err != nil {
_ = windows.CloseHandle(h)
return "", nil, err
}
return realSrc, func() error { return windows.CloseHandle(h) }, nil
}
func verifySubpathWithinRoot(root, realSrc, subPath string) error {
realRoot, err := resolveFinalPath(root)
if err != nil {
return errors.Wrapf(err, "resolving mount root %q", root)
}
if !pathWithinRoot(realRoot, realSrc) {
return errors.Errorf("mount source subpath %q resolves to %q which escapes the mount root %q", subPath, realSrc, realRoot)
}
return nil
}
// resolveFinalPath returns the real path of p with all reparse points followed.
func resolveFinalPath(p string) (string, error) {
pathPtr, err := windows.UTF16PtrFromString(p)
if err != nil {
return "", err
}
// FILE_FLAG_BACKUP_SEMANTICS allows opening a directory; omitting
// FILE_FLAG_OPEN_REPARSE_POINT makes GetFinalPathNameByHandle follow links.
h, err := windows.CreateFile(
pathPtr,
0,
windows.FILE_SHARE_READ|windows.FILE_SHARE_WRITE|windows.FILE_SHARE_DELETE,
nil,
windows.OPEN_EXISTING,
windows.FILE_FLAG_BACKUP_SEMANTICS,
0,
)
if err != nil {
return "", err
}
defer windows.CloseHandle(h)
return finalPathNameByHandle(h)
}
func finalPathNameByHandle(h windows.Handle) (string, error) {
// flags 0 == FILE_NAME_NORMALIZED | VOLUME_NAME_DOS.
n := uint32(windows.MAX_PATH)
for {
buf := make([]uint16, n)
got, err := windows.GetFinalPathNameByHandle(h, &buf[0], n, 0)
if err != nil {
return "", err
}
if got <= n {
return windows.UTF16ToString(buf[:got]), nil
}
n = got
}
}
func mountSourcePath(p string) string {
if strings.HasPrefix(p, `\\?\UNC\`) {
return `\\` + p[len(`\\?\UNC\`):]
}
if len(p) >= len(`\\?\C:\`) && p[:4] == `\\?\` && isDriveLetter(p[4]) && p[5:7] == `:\` {
return p[4:]
}
return p
}
func isDriveLetter(c byte) bool {
return ('a' <= c && c <= 'z') || ('A' <= c && c <= 'Z')
}
// pathWithinRoot reports whether p is root or a descendant of root. Inputs are
// already-resolved real paths; filepath.Rel handles case-insensitivity and
// cross-volume paths on Windows.
func pathWithinRoot(root, p string) bool {
rel, err := filepath.Rel(root, p)
if err != nil {
return false
}
return rel == "." || (rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator)))
}
func generateLinuxResourceOpts(res *pb.LinuxResources) ([]oci.SpecOpts, error) {

View File

@@ -0,0 +1,232 @@
//go:build windows
package oci
import (
"os"
"os/exec"
"path/filepath"
"strings"
"testing"
"github.com/containerd/containerd/v2/core/mount"
"github.com/stretchr/testify/require"
)
// mklinkJunction creates a Windows directory junction (mklink /J).
func mklinkJunction(t *testing.T, link, target string) {
t.Helper()
out, err := exec.CommandContext(t.Context(), "cmd", "/c", "mklink", "/J", link, target).CombinedOutput()
require.NoErrorf(t, err, "mklink /J %q %q failed: %s", link, target, out)
}
// mklinkSymlinkDir creates a Windows directory symlink (mklink /D). It may need
// Developer Mode or SeCreateSymbolicLinkPrivilege; callers should skip on error.
func mklinkSymlinkDir(t *testing.T, link, target string) error {
t.Helper()
out, err := exec.CommandContext(t.Context(), "cmd", "/c", "mklink", "/D", link, target).CombinedOutput()
if err != nil {
t.Logf("mklink /D %q %q failed (symlink creation may be unprivileged): %s", link, target, out)
}
return err
}
// TestSubResolvesBenignSubdir checks that a normal cache subdirectory resolves
// to a path inside the cache root.
func TestSubResolvesBenignSubdir(t *testing.T) {
cacheRoot := t.TempDir()
sel := filepath.Join(cacheRoot, "sel")
require.NoError(t, os.MkdirAll(sel, 0700))
require.NoError(t, os.WriteFile(filepath.Join(sel, "benign.txt"), []byte("benign"), 0600))
m, cleanup, err := sub(mount.Mount{Source: cacheRoot}, "sel")
require.NoError(t, err)
if cleanup != nil {
defer cleanup()
}
realSel, err := resolveFinalPath(sel)
require.NoError(t, err)
realMountSource, err := resolveFinalPath(m.Source)
require.NoError(t, err)
require.Equal(t, realSel, realMountSource, "benign subdir must resolve to the cache subdirectory")
require.False(t, strings.HasPrefix(m.Source, `\\?\`), "mount source should use the plain DOS path form when available")
}
// TestSubRejectsJunctionEscape is the core regression test: a cache subdir
// replaced by a junction pointing outside the cache root must be rejected.
func TestSubRejectsJunctionEscape(t *testing.T) {
outside := t.TempDir()
require.NoError(t, os.WriteFile(filepath.Join(outside, "marker.txt"), []byte("OUTSIDE"), 0600))
cacheRoot := t.TempDir()
sel := filepath.Join(cacheRoot, "sel")
mklinkJunction(t, sel, outside)
m, cleanup, err := sub(mount.Mount{Source: cacheRoot}, "sel")
if cleanup != nil {
defer cleanup()
}
require.Error(t, err, "sub() must reject a junction that escapes the cache root")
require.NotEqual(t, outside, m.Source, "resolved source must not be the outside host path")
}
// TestSubRejectsSymlinkEscape verifies the same protection for a directory
// symbolic link (mklink /D) pointing outside the cache root.
func TestSubRejectsSymlinkEscape(t *testing.T) {
outside := t.TempDir()
require.NoError(t, os.WriteFile(filepath.Join(outside, "marker.txt"), []byte("OUTSIDE"), 0600))
cacheRoot := t.TempDir()
sel := filepath.Join(cacheRoot, "sel")
if err := mklinkSymlinkDir(t, sel, outside); err != nil {
t.Skipf("could not create directory symlink (insufficient privilege?): %v", err)
}
m, cleanup, err := sub(mount.Mount{Source: cacheRoot}, "sel")
if cleanup != nil {
defer cleanup()
}
require.Error(t, err, "sub() must reject a symlink that escapes the cache root")
require.NotEqual(t, outside, m.Source, "resolved source must not be the outside host path")
}
// TestSubRejectsNestedJunctionEscape checks a junction in a non-terminal path
// component (source "mid/leaf" where "mid" escapes the root) is also rejected.
func TestSubRejectsNestedJunctionEscape(t *testing.T) {
outside := t.TempDir()
require.NoError(t, os.MkdirAll(filepath.Join(outside, "leaf"), 0700))
require.NoError(t, os.WriteFile(filepath.Join(outside, "leaf", "marker.txt"), []byte("OUTSIDE"), 0600))
cacheRoot := t.TempDir()
mid := filepath.Join(cacheRoot, "mid")
mklinkJunction(t, mid, outside)
m, cleanup, err := sub(mount.Mount{Source: cacheRoot}, "mid/leaf")
if cleanup != nil {
defer cleanup()
}
require.Error(t, err, "sub() must reject a path that traverses a junction outside the cache root")
require.NotEqual(t, filepath.Join(outside, "leaf"), m.Source, "resolved source must not escape the cache root")
}
// TestSubPinsSourceAgainstSwap verifies that while sub() holds the mount source,
// the entry cannot be renamed/swapped, and that releasing the cleanup lifts it.
func TestSubPinsSourceAgainstSwap(t *testing.T) {
cacheRoot := t.TempDir()
sel := filepath.Join(cacheRoot, "sel")
require.NoError(t, os.MkdirAll(sel, 0700))
_, cleanup, err := sub(mount.Mount{Source: cacheRoot}, "sel")
require.NoError(t, err)
require.NotNil(t, cleanup)
require.Error(t, os.Rename(sel, sel+".swap"), "pinned source must not be renamable during the mount window")
require.NoError(t, cleanup())
require.NoError(t, os.Rename(sel, sel+".swap"), "source must be renamable after the pin is released")
}
// TestSubPinAllowsChildChanges verifies that the restrictive share mode applies
// to the selected directory object without making the mounted cache contents
// read-only.
func TestSubPinAllowsChildChanges(t *testing.T) {
cacheRoot := t.TempDir()
sel := filepath.Join(cacheRoot, "sel")
require.NoError(t, os.MkdirAll(sel, 0700))
_, cleanup, err := sub(mount.Mount{Source: cacheRoot}, "sel")
require.NoError(t, err)
require.NotNil(t, cleanup)
defer cleanup()
child := filepath.Join(sel, "child")
require.NoError(t, os.WriteFile(child, []byte("first"), 0600))
require.NoError(t, os.WriteFile(child, []byte("second"), 0600))
renamed := filepath.Join(sel, "renamed")
require.NoError(t, os.Rename(child, renamed))
require.NoError(t, os.Remove(renamed))
nested := filepath.Join(sel, "nested")
require.NoError(t, os.Mkdir(nested, 0700))
require.NoError(t, os.Remove(nested))
}
// TestSubUsesPinnedResolvedPath verifies that HCS receives the resolved target
// path rather than the original path containing an attacker-controlled
// junction. Replacing the original junction therefore cannot redirect the
// later mount.
func TestSubUsesPinnedResolvedPath(t *testing.T) {
cacheRoot := t.TempDir()
inside := filepath.Join(cacheRoot, "inside")
require.NoError(t, os.MkdirAll(inside, 0700))
sel := filepath.Join(cacheRoot, "sel")
mklinkJunction(t, sel, inside)
m, cleanup, err := sub(mount.Mount{Source: cacheRoot}, "sel")
require.NoError(t, err)
require.NotNil(t, cleanup)
defer cleanup()
realInside, err := resolveFinalPath(inside)
require.NoError(t, err)
realMountSource, err := resolveFinalPath(m.Source)
require.NoError(t, err)
require.Equal(t, realInside, realMountSource)
require.False(t, strings.HasPrefix(m.Source, `\\?\`), "mount source should use the plain DOS path form when available")
require.NoError(t, os.Remove(sel), "the original junction is not the pinned source object")
outside := t.TempDir()
mklinkJunction(t, sel, outside)
realMountSource, err = resolveFinalPath(m.Source)
require.NoError(t, err)
require.Equal(t, realInside, realMountSource, "replacing the original junction must not redirect the mount source")
}
func TestMountSourcePath(t *testing.T) {
cases := []struct {
name string
in string
want string
}{
{"drive letter", `\\?\C:\cache\sel`, `C:\cache\sel`},
{"unc", `\\?\UNC\server\share\cache\sel`, `\\server\share\cache\sel`},
{"volume guid", `\\?\Volume{11111111-1111-1111-1111-111111111111}\cache\sel`, `\\?\Volume{11111111-1111-1111-1111-111111111111}\cache\sel`},
{"plain drive letter", `C:\cache\sel`, `C:\cache\sel`},
{"similar prefix", `\\x\C:\cache\sel`, `\\x\C:\cache\sel`},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
require.Equal(t, tc.want, mountSourcePath(tc.in))
})
}
}
func TestPathWithinRoot(t *testing.T) {
cases := []struct {
name string
root string
p string
want bool
}{
{"equal", `C:\cache`, `C:\cache`, true},
{"equal trailing sep on p", `C:\cache`, `C:\cache\`, true},
{"equal trailing sep on root", `C:\cache\`, `C:\cache`, true},
{"direct child", `C:\cache`, `C:\cache\sel`, true},
{"nested child", `C:\cache`, `C:\cache\a\b`, true},
{"case-insensitive child", `C:\Cache`, `c:\cache\sel`, true},
{"child name starting with dotdot", `C:\cache`, `C:\cache\..foo`, true},
{"exact parent", `C:\cache`, `C:\`, false},
{"sibling prefix confusion", `C:\cache`, `C:\cache-evil`, false},
{"unrelated escape", `C:\cache`, `C:\Windows`, false},
{"different volume", `C:\cache`, `D:\cache\sel`, false},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
require.Equal(t, tc.want, pathWithinRoot(tc.root, tc.p))
})
}
}

View File

@@ -29,6 +29,7 @@ var mountTests = integration.TestFuncs(
testMountDuplicate,
testCacheMountUser,
testCacheMountParallel,
testCacheMountSourceEscape,
)
func init() {
@@ -699,3 +700,57 @@ COPY --from=b2 /License.txt p2
require.NoError(t, err)
}
}
func testCacheMountSourceEscape(t *testing.T, sb integration.Sandbox) {
f := getFrontend(t, sb)
dockerfile := []byte(integration.UnixOrWindows(
`
FROM busybox AS init
RUN --mount=type=cache,id=sourceescape,target=/cache mkdir -p /cache/sel && echo benign > /cache/sel/benign.txt
FROM init AS link
RUN --mount=type=cache,id=sourceescape,target=/cache rm -rf /cache/sel && ln -s /etc /cache/sel
FROM link AS victim
RUN --mount=type=cache,id=sourceescape,source=sel,target=/victim cat /victim/hostname
FROM scratch
COPY --from=victim /etc/hostname /hostname
`,
`# escape=`+"`"+`
FROM nanoserver AS init
USER ContainerAdministrator
RUN --mount=type=cache,id=sourceescape,target=C:\cache cmd /S /C "if not exist C:\cache\sel mkdir C:\cache\sel & echo benign> C:\cache\sel\benign.txt"
FROM init AS link
RUN --mount=type=cache,id=sourceescape,target=C:\cache cmd /S /C "rmdir C:\cache\sel /S /Q & mklink /J C:\cache\sel C:\Windows"
FROM link AS victim
RUN --mount=type=cache,id=sourceescape,source=sel,target=C:\victim cmd /S /C "dir C:\victim"
FROM nanoserver AS final
COPY --from=victim C:\License.txt C:\License.txt
`,
))
dir := integration.Tmpdir(
t,
fstest.CreateFile("Dockerfile", dockerfile, 0600),
)
c, err := client.New(sb.Context(), sb.Address())
require.NoError(t, err)
defer c.Close()
_, err = f.Solve(sb.Context(), c, client.SolveOpt{
FrontendAttrs: map[string]string{
"no-cache": "",
},
LocalMounts: map[string]fsutil.FS{
dockerui.DefaultLocalNameDockerfile: dir,
dockerui.DefaultLocalNameContext: dir,
},
}, nil)
require.Error(t, err, "build must fail when a cache source= resolves through a link outside the cache root")
}

2
go.mod
View File

@@ -78,7 +78,7 @@ require (
github.com/spdx/tools-golang v0.5.7
github.com/stretchr/testify v1.11.1
github.com/tonistiigi/dchapes-mode v0.0.0-20250318174251-73d941a28323
github.com/tonistiigi/fsutil v0.0.0-20260609091201-0257b3308df4
github.com/tonistiigi/fsutil v0.0.0-20260716115106-30cd4fc5d911 // buildkit-v0.31
github.com/tonistiigi/go-actions-cache v0.0.0-20260120203934-54bc28c26fd2
github.com/tonistiigi/go-archvariant v1.0.0
github.com/tonistiigi/go-csvvalue v0.0.0-20240814133006-030d3b2625d0

4
go.sum
View File

@@ -584,8 +584,8 @@ github.com/titanous/rocacheck v0.0.0-20171023193734-afe73141d399 h1:e/5i7d4oYZ+C
github.com/titanous/rocacheck v0.0.0-20171023193734-afe73141d399/go.mod h1:LdwHTNJT99C5fTAzDz0ud328OgXz+gierycbcIx2fRs=
github.com/tonistiigi/dchapes-mode v0.0.0-20250318174251-73d941a28323 h1:r0p7fK56l8WPequOaR3i9LBqfPtEdXIQbUTzT55iqT4=
github.com/tonistiigi/dchapes-mode v0.0.0-20250318174251-73d941a28323/go.mod h1:3Iuxbr0P7D3zUzBMAZB+ois3h/et0shEz0qApgHYGpY=
github.com/tonistiigi/fsutil v0.0.0-20260609091201-0257b3308df4 h1:tJkv/edHw9FXVtbHxc6cpqDttiCLNzhqI1W40fcnxIY=
github.com/tonistiigi/fsutil v0.0.0-20260609091201-0257b3308df4/go.mod h1:K5zrLch9UaSGNiek5XHZeqZUf1zPWJHqDfLIcnpquQ4=
github.com/tonistiigi/fsutil v0.0.0-20260716115106-30cd4fc5d911 h1:xJZz1fhsRSrGTzQ6wvh1gX6d5jQaYjbIuGXT2s0AWuM=
github.com/tonistiigi/fsutil v0.0.0-20260716115106-30cd4fc5d911/go.mod h1:K5zrLch9UaSGNiek5XHZeqZUf1zPWJHqDfLIcnpquQ4=
github.com/tonistiigi/go-actions-cache v0.0.0-20260120203934-54bc28c26fd2 h1:5p6hffZeB25G4rhBc3HU6x1aIlyDELfib+/Omq+ZfQA=
github.com/tonistiigi/go-actions-cache v0.0.0-20260120203934-54bc28c26fd2/go.mod h1:cD0SB2270BYw6HYKriFn4H6NRLhGj6ytf48YTpsm8LY=
github.com/tonistiigi/go-archvariant v1.0.0 h1:5LC1eDWiBNflnTF1prCiX09yfNHIxDC/aukdhCdTyb0=

View File

@@ -354,9 +354,14 @@ func (sb *subBuilder) InContext(ctx context.Context, f func(context.Context, Job
}
func (sb *subBuilder) EachValue(ctx context.Context, key string, fn func(any) error) error {
sb.mu.Lock()
defer sb.mu.Unlock()
sb.state.mu.Lock()
jobs := make([]*Job, 0, len(sb.jobs))
for j := range sb.jobs {
jobs = append(jobs, j)
}
sb.state.mu.Unlock()
for _, j := range jobs {
if err := j.EachValue(ctx, key, fn); err != nil {
return err
}

View File

@@ -0,0 +1,55 @@
package llbsolver
import (
"testing"
"github.com/moby/buildkit/solver"
"github.com/moby/buildkit/solver/pb"
digest "github.com/opencontainers/go-digest"
"github.com/stretchr/testify/require"
)
func noopLoad(string) (solver.Vertex, error) { return nil, nil }
// llbOpName previously panicked on malicious diff ops (nil lower/upper input,
// out-of-range input index). It must now return an error instead.
func TestDiffNilLowerName(t *testing.T) {
pbOp := &pb.Op{
Op: &pb.Op_Diff{Diff: &pb.DiffOp{Lower: nil, Upper: &pb.UpperDiffInput{Input: -1}}},
Inputs: []*pb.Input{},
}
require.NotPanics(t, func() {
_, err := llbOpName(pbOp, noopLoad)
require.Error(t, err)
})
}
func TestDiffOOBInputName(t *testing.T) {
pbOp := &pb.Op{
Op: &pb.Op_Diff{Diff: &pb.DiffOp{
Lower: &pb.LowerDiffInput{Input: 5},
Upper: &pb.UpperDiffInput{Input: -1},
}},
Inputs: []*pb.Input{{Digest: string(digest.FromBytes([]byte("x")))}},
}
require.NotPanics(t, func() {
_, err := llbOpName(pbOp, noopLoad)
require.Error(t, err)
})
}
func TestDiffInputWithoutInputsName(t *testing.T) {
// input index 0 referenced but no inputs declared.
pbOp := &pb.Op{
Op: &pb.Op_Diff{Diff: &pb.DiffOp{
Lower: &pb.LowerDiffInput{Input: 0},
Upper: &pb.UpperDiffInput{Input: -1},
}},
Inputs: []*pb.Input{},
}
require.NotPanics(t, func() {
_, err := llbOpName(pbOp, noopLoad)
require.Error(t, err)
})
}

View File

@@ -162,12 +162,12 @@ func rm(d string, action *pb.FileActionRm) (err error) {
}
func rmPath(root, src string, allowNotFound bool) error {
src = filepath.Clean(src)
src = filepath.Join("/", src)
dir, base := filepath.Split(src)
if base == "" {
return errors.New("rmPath: invalid empty path")
}
dir, err := fs.RootPath(root, filepath.Join("/", dir))
dir, err := fs.RootPath(root, dir)
if err != nil {
return errors.WithStack(err)
}

View File

@@ -5,6 +5,7 @@ import (
"path/filepath"
"testing"
"github.com/moby/buildkit/solver/pb"
"github.com/pkg/errors"
"github.com/stretchr/testify/require"
)
@@ -35,3 +36,45 @@ func TestRmPathFileExists(t *testing.T) {
require.True(t, os.IsNotExist(err))
}
func TestRmParentTraversalDoesNotEscapeRoot(t *testing.T) {
// Backslash variants are separators on Windows (real traversal there) and
// ordinary filename characters on Linux (inert, but still must not escape).
for _, p := range []string{
"..", "../..", "a/../..", "../victim",
"..\\..", "a\\..\\..", "..\\victim",
} {
t.Run(p, func(t *testing.T) {
parent := t.TempDir()
root := filepath.Join(parent, "root")
victim := filepath.Join(parent, "victim")
require.NoError(t, os.Mkdir(root, 0o755))
require.NoError(t, os.Mkdir(victim, 0o755))
require.NoError(t, os.WriteFile(filepath.Join(victim, "data"), []byte("data"), 0o644))
require.Error(t, rm(root, &pb.FileActionRm{Path: p}))
_, err := os.Stat(victim)
require.NoError(t, err, "rm escaped root and deleted sibling %q", victim)
})
}
}
func TestRmPathRemovesSymlinkItself(t *testing.T) {
root := t.TempDir()
target := filepath.Join(root, "target")
link := filepath.Join(root, "link")
require.NoError(t, os.WriteFile(target, []byte("target"), 0o644))
require.NoError(t, os.Symlink("target", link))
require.NoError(t, rmPath(root, "link", false))
_, err := os.Lstat(link)
require.True(t, os.IsNotExist(err))
_, err = os.Stat(target)
require.NoError(t, err)
}

View File

@@ -77,7 +77,7 @@ func (b *BuildOp) Exec(ctx context.Context, job solver.JobContext, inputs []solv
}
i := int(llbDef.Input)
if i >= len(inputs) {
if i < 0 || i >= len(inputs) {
return nil, errors.Errorf("invalid index %v", i) // TODO: this should be validated before
}
inp := inputs[i]

View File

@@ -70,6 +70,9 @@ func (d *diffOp) Exec(ctx context.Context, jobCtx solver.JobContext, inputs []so
var lowerRef cache.ImmutableRef
if d.op.Lower.Input != int64(pb.Empty) {
if curInput >= len(inputs) {
return nil, errors.Errorf("invalid lower input index %d for diff op with %d inputs", curInput, len(inputs))
}
if lowerInp := inputs[curInput]; lowerInp != nil {
wref, ok := lowerInp.Sys().(*worker.WorkerRef)
if !ok {
@@ -84,6 +87,9 @@ func (d *diffOp) Exec(ctx context.Context, jobCtx solver.JobContext, inputs []so
var upperRef cache.ImmutableRef
if d.op.Upper.Input != int64(pb.Empty) {
if curInput >= len(inputs) {
return nil, errors.Errorf("invalid upper input index %d for diff op with %d inputs", curInput, len(inputs))
}
if upperInp := inputs[curInput]; upperInp != nil {
wref, ok := upperInp.Sys().(*worker.WorkerRef)
if !ok {

View File

@@ -302,7 +302,7 @@ func (e *ExecOp) getMountDeps() ([]dep, error) {
if m.Input == int64(pb.Empty) {
continue
}
if int(m.Input) >= len(deps) {
if m.Input < 0 || int(m.Input) >= len(deps) {
return nil, errors.Errorf("invalid mountinput %v", m)
}
@@ -394,7 +394,7 @@ func (e *ExecOp) Exec(ctx context.Context, jobCtx solver.JobContext, inputs []so
if err != nil {
execInputs := make([]solver.Result, len(e.op.Mounts))
for i, m := range e.op.Mounts {
if m.Input == -1 {
if m.Input < 0 || int(m.Input) >= len(inputs) {
continue
}
execInputs[i] = inputs[m.Input].Clone()

View File

@@ -149,6 +149,9 @@ func (f *fileOp) CacheMap(ctx context.Context, jobCtx solver.JobContext, index i
}
for idx, m := range selectors {
if idx < 0 || idx >= len(cm.Deps) {
return nil, false, errors.Errorf("invalid input index %d in file op with %d inputs", idx, len(cm.Deps))
}
if _, ok := invalidSelectors[idx]; ok {
continue
}

View File

@@ -0,0 +1,99 @@
package ops
import (
"testing"
"github.com/moby/buildkit/solver/pb"
"github.com/stretchr/testify/require"
)
// These exercise malicious op input indices that previously panicked the
// daemon (out-of-range / negative slice indexing). They must now return an
// error instead of panicking.
func TestExecMountNegativeInput(t *testing.T) {
op := &ExecOp{
op: &pb.ExecOp{
Meta: &pb.Meta{Args: []string{"x"}},
Mounts: []*pb.Mount{{Dest: "/", Input: -2}},
},
numInputs: 1,
}
require.NotPanics(t, func() {
_, _, err := op.CacheMap(t.Context(), testJobContext(t), 1)
require.Error(t, err)
})
}
func TestFileCopySecondaryNegativeInput(t *testing.T) {
fo := &pb.FileOp{
Actions: []*pb.FileAction{
{
Input: 0,
SecondaryInput: -2,
Output: 0,
Action: &pb.FileAction_Copy{
Copy: &pb.FileActionCopy{Src: "/src", Dest: "/dest"},
},
},
},
}
f := &fileOp{op: fo, numInputs: 1}
require.NotPanics(t, func() {
_, _, err := f.CacheMap(t.Context(), testJobContext(t), 1)
require.Error(t, err)
})
}
func TestFileOwnerUnboundedInput(t *testing.T) {
fo := &pb.FileOp{
Actions: []*pb.FileAction{
{
Input: 0,
SecondaryInput: -1,
Output: 0,
Action: &pb.FileAction_Mkdir{
Mkdir: &pb.FileActionMkDir{
Path: "/foo",
Mode: 0700,
Owner: &pb.ChownOpt{
User: &pb.UserOpt{User: &pb.UserOpt_ByName{ByName: &pb.NamedUserOpt{Input: 1000000}}},
},
},
},
},
},
}
f := &fileOp{op: fo, numInputs: 1}
require.NotPanics(t, func() {
_, _, err := f.CacheMap(t.Context(), testJobContext(t), 1)
require.Error(t, err)
})
}
func TestFileOwnerInputWithoutInputs(t *testing.T) {
// numInputs == 0 edge: owner references input 0 but there are no inputs.
fo := &pb.FileOp{
Actions: []*pb.FileAction{
{
Input: -1,
SecondaryInput: -1,
Output: 0,
Action: &pb.FileAction_Mkdir{
Mkdir: &pb.FileActionMkDir{
Path: "/foo",
Mode: 0700,
Owner: &pb.ChownOpt{
User: &pb.UserOpt{User: &pb.UserOpt_ByName{ByName: &pb.NamedUserOpt{Input: 0}}},
},
},
},
},
},
}
f := &fileOp{op: fo, numInputs: 0}
require.NotPanics(t, func() {
_, _, err := f.CacheMap(t.Context(), testJobContext(t), 1)
require.Error(t, err)
})
}

View File

@@ -59,6 +59,9 @@ func Validate(op *pb.Op) error {
if op.Diff == nil {
return errors.Errorf("invalid nil diff op")
}
if op.Diff.Lower == nil || op.Diff.Upper == nil {
return errors.Errorf("invalid diff op with nil lower or upper input")
}
case *pb.Op_Passthrough:
if op.Passthrough == nil {
return errors.Errorf("invalid nil passthrough op")

View File

@@ -0,0 +1,14 @@
package opsutils
import (
"testing"
"github.com/moby/buildkit/solver/pb"
"github.com/stretchr/testify/require"
)
func TestValidateDiffNilInputs(t *testing.T) {
require.Error(t, Validate(&pb.Op{Op: &pb.Op_Diff{Diff: &pb.DiffOp{Lower: nil, Upper: &pb.UpperDiffInput{Input: -1}}}}))
require.Error(t, Validate(&pb.Op{Op: &pb.Op_Diff{Diff: &pb.DiffOp{Lower: &pb.LowerDiffInput{Input: -1}, Upper: nil}}}))
require.NoError(t, Validate(&pb.Op{Op: &pb.Op_Diff{Diff: &pb.DiffOp{Lower: &pb.LowerDiffInput{Input: -1}, Upper: &pb.UpperDiffInput{Input: -1}}}}))
}

View File

@@ -367,6 +367,11 @@ func loadLLB(ctx context.Context, def *pb.Definition, polEngine SourcePolicyEval
if err := pbop.Unmarshal(dt); err != nil {
return solver.Edge{}, errors.Wrap(err, "failed to parse llb proto op")
}
for i, input := range pbop.Inputs {
if input.Index < 0 {
return solver.Edge{}, errors.Errorf("invalid input %d output index %d", i, input.Index)
}
}
dgst := digest.FromBytes(dt)
if pbop.GetSource() != nil {
sources[dgst] = struct{}{}
@@ -482,25 +487,16 @@ func llbOpName(pbOp *pb.Op, load func(string) (solver.Vertex, error)) (string, e
}
return "merge " + fmt.Sprintf("(%s)", strings.Join(subnames, ", ")), nil
case *pb.Op_Diff:
var lowerName string
if op.Diff.Lower.Input == -1 {
lowerName = "scratch"
} else {
lowerVtx, err := load(pbOp.Inputs[op.Diff.Lower.Input].Digest)
if err != nil {
return "", err
}
lowerName = fmt.Sprintf("(%s)", lowerVtx.Name())
if op.Diff.Lower == nil || op.Diff.Upper == nil {
return "", errors.Errorf("invalid diff op with nil lower or upper input")
}
var upperName string
if op.Diff.Upper.Input == -1 {
upperName = "scratch"
} else {
upperVtx, err := load(pbOp.Inputs[op.Diff.Upper.Input].Digest)
if err != nil {
return "", err
}
upperName = fmt.Sprintf("(%s)", upperVtx.Name())
lowerName, err := diffSideName(pbOp, op.Diff.Lower.Input, load)
if err != nil {
return "", err
}
upperName, err := diffSideName(pbOp, op.Diff.Upper.Input, load)
if err != nil {
return "", err
}
return "diff " + lowerName + " -> " + upperName, nil
case *pb.Op_Passthrough:
@@ -510,6 +506,20 @@ func llbOpName(pbOp *pb.Op, load func(string) (solver.Vertex, error)) (string, e
}
}
func diffSideName(pbOp *pb.Op, input int64, load func(string) (solver.Vertex, error)) (string, error) {
if input == int64(pb.Empty) {
return "scratch", nil
}
if input < 0 || int(input) >= len(pbOp.Inputs) {
return "", errors.Errorf("invalid diff input index %d", input)
}
vtx, err := load(pbOp.Inputs[input].Digest)
if err != nil {
return "", err
}
return fmt.Sprintf("(%s)", vtx.Name()), nil
}
func fileOpName(actions []*pb.FileAction) string {
names := make([]string, 0, len(actions))
for _, action := range actions {

View File

@@ -199,6 +199,30 @@ func TestBridgeUsesDefaultProxyNetwork(t *testing.T) {
require.True(t, br.proxyNetwork)
}
func TestLoadRejectsNegativeInputIndex(t *testing.T) {
for _, tt := range []struct {
name string
execInputIndex int64
rootInputIndex int64
}{
{
name: "vertex input",
execInputIndex: -1,
rootInputIndex: 0,
},
{
name: "root input",
execInputIndex: 0,
rootInputIndex: -1,
},
} {
t.Run(tt.name, func(t *testing.T) {
_, err := Load(t.Context(), inputIndexTestDefinition(t, tt.execInputIndex, tt.rootInputIndex), nil)
require.ErrorContains(t, err, "invalid input 0 output index -1")
})
}
}
func proxyNetworkTestDefinition(t *testing.T, opts ...func(*pb.ExecOp)) *pb.Definition {
t.Helper()
source := &pb.Op{
@@ -233,6 +257,37 @@ func proxyNetworkTestDefinition(t *testing.T, opts ...func(*pb.ExecOp)) *pb.Defi
return &pb.Definition{Def: [][]byte{sourceBytes, execBytes, rootBytes}}
}
func inputIndexTestDefinition(t *testing.T, execInputIndex, rootInputIndex int64) *pb.Definition {
t.Helper()
source := &pb.Op{
Op: &pb.Op_Source{
Source: &pb.SourceOp{Identifier: "local://context"},
},
}
sourceDigest, sourceBytes := marshalTestOp(t, source)
exec := &pb.Op{
Inputs: []*pb.Input{{Digest: string(sourceDigest), Index: execInputIndex}},
Op: &pb.Op_Exec{
Exec: &pb.ExecOp{
Meta: &pb.Meta{Args: []string{"true"}},
Mounts: []*pb.Mount{{
Input: 0,
Dest: pb.RootMount,
}},
},
},
}
execDigest, execBytes := marshalTestOp(t, exec)
root := &pb.Op{
Inputs: []*pb.Input{{Digest: string(execDigest), Index: rootInputIndex}},
}
_, rootBytes := marshalTestOp(t, root)
return &pb.Definition{Def: [][]byte{sourceBytes, execBytes, rootBytes}}
}
func marshalTestOp(t *testing.T, op *pb.Op) (digest.Digest, []byte) {
t.Helper()
dt, err := op.Marshal()

View File

@@ -398,8 +398,8 @@ func (gs *gitSourceHandler) checkoutAsBundle(ctx context.Context, repo *gitRepo,
// natural-name ref to pull; update-ref alone places the tip against
// the pinned commit under the target name.
sharedURL := "file://" + repo.dir
if _, err := tmpGit.Run(ctx, "fetch", sharedURL, commit); err != nil {
return nil, errors.Wrapf(err, "failed to fetch commit %s from shared repo for bundle creation", commit)
if err := fetchCommitForBundle(ctx, tmpGit, sharedURL, commit); err != nil {
return nil, err
}
if _, err := tmpGit.Run(ctx, "update-ref", targetRef, commit); err != nil {
@@ -441,6 +441,13 @@ func (gs *gitSourceHandler) checkoutAsBundle(ctx context.Context, repo *gitRepo,
return snap, nil
}
func fetchCommitForBundle(ctx context.Context, git *gitutil.GitCLI, sharedURL, commit string) error {
if _, err := git.Run(ctx, "fetch", sharedURL, "--", commit); err != nil {
return errors.Wrapf(err, "failed to fetch commit %s from shared repo for bundle creation", commit)
}
return nil
}
// writeBundleToMount copies the bytes at stagePath into
// <checkoutDir>/<bundleName> via os.OpenRoot, so a symlink at the destination
// cannot redirect the write outside checkoutDir. The staging file is expected

View File

@@ -1557,7 +1557,11 @@ func getDefaultBranch(ctx context.Context, git *gitutil.GitCLI, remoteURL string
if len(ss) == 0 || len(ss[0]) != 2 {
return "", errors.Errorf("could not find default branch for repository: %s", urlutil.RedactCredentials(remoteURL))
}
return ss[0][1], nil
ref := ss[0][1]
if err := validateGitRef(ref); err != nil {
return "", err
}
return ref, nil
}
const (

View File

@@ -2815,6 +2815,79 @@ func TestBundleStagedRefShape(t *testing.T) {
require.Equal(t, headSha, md.Checksum)
}
func TestGetDefaultBranchRejectsDashPrefixedRef(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("Depends on git shell helpers")
}
t.Parallel()
ctx := t.Context()
work := t.TempDir()
runShell(t, work,
"git init -q seed",
"git -C seed -c user.email=test@example.com -c user.name=test commit -q --allow-empty -m seed",
"git clone -q --bare seed mal.git",
)
shaCmd := exec.CommandContext(ctx, "git", "-C", filepath.Join(work, "seed"), "rev-parse", "HEAD") //nolint:gosec // test uses t.TempDir-owned path
out, err := shaCmd.Output()
require.NoError(t, err)
sha := strings.TrimSpace(string(out))
malDir := filepath.Join(work, "mal.git")
require.NoError(t, os.RemoveAll(filepath.Join(malDir, "refs", "heads")))
require.NoError(t, os.WriteFile(filepath.Join(malDir, "packed-refs"),
fmt.Appendf(nil, "# pack-refs with: peeled fully-peeled sorted\n%s refs/heads/--upload-pack=/tmp/evil.sh\n", sha), 0600))
require.NoError(t, os.WriteFile(filepath.Join(malDir, "HEAD"),
[]byte("ref: refs/heads/--upload-pack=/tmp/evil.sh\n"), 0600))
ref, err := getDefaultBranch(ctx, gitutil.NewGitCLI(), "file://"+malDir)
require.ErrorContains(t, err, `invalid git ref "--upload-pack=/tmp/evil.sh"`)
require.Empty(t, ref)
}
func TestFetchCommitForBundleUsesOptionTerminator(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("Depends on shell script git stub")
}
t.Parallel()
ctx := t.Context()
dir := t.TempDir()
capture := filepath.Join(dir, "args")
fakeGit := filepath.Join(dir, "git")
require.NoError(t, os.WriteFile(fakeGit, fmt.Appendf(nil, `#!/bin/sh
for arg do
printf '%%s\n' "$arg"
done > %s
`, strconv.Quote(capture)), 0600))
require.NoError(t, os.Chmod(fakeGit, 0700))
sharedURL := "file:///tmp/shared.git"
commit := "--upload-pack=/tmp/evil.sh"
err := fetchCommitForBundle(ctx, gitutil.NewGitCLI(gitutil.WithGitBinary(fakeGit)), sharedURL, commit)
require.NoError(t, err)
buf, err := os.ReadFile(capture)
require.NoError(t, err)
args := strings.Split(strings.TrimSpace(string(buf)), "\n")
fetchIdx := -1
for i, arg := range args {
if arg == "fetch" {
fetchIdx = i
break
}
}
require.NotEqual(t, -1, fetchIdx, "fake git args: %v", args)
require.GreaterOrEqual(t, len(args), fetchIdx+4, "fake git args: %v", args)
require.Equal(t, sharedURL, args[fetchIdx+1])
require.Equal(t, "--", args[fetchIdx+2])
require.Equal(t, commit, args[fetchIdx+3])
}
func TestDetectBundleSHA256(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("Depends on git bundle support exercised via shell helpers")

View File

@@ -38,7 +38,7 @@ func (v *Validator) HandleChange(kind ChangeKind, p string, fi os.FileInfo, err
if dir == "." {
dir = ""
}
if dir == ".." || strings.HasPrefix(p, filepath.FromSlash("../")) {
if p == ".." || dir == ".." || strings.HasPrefix(p, filepath.FromSlash("../")) {
return errors.WithStack(&os.PathError{Path: p, Err: syscall.EINVAL, Op: "escape check"})
}

2
vendor/modules.txt vendored
View File

@@ -1038,7 +1038,7 @@ github.com/theupdateframework/go-tuf/v2/metadata/updater
# github.com/tonistiigi/dchapes-mode v0.0.0-20250318174251-73d941a28323
## explicit; go 1.21
github.com/tonistiigi/dchapes-mode
# github.com/tonistiigi/fsutil v0.0.0-20260609091201-0257b3308df4
# github.com/tonistiigi/fsutil v0.0.0-20260716115106-30cd4fc5d911
## explicit; go 1.25.0
github.com/tonistiigi/fsutil
github.com/tonistiigi/fsutil/copy