Check the validity of the chmod option arguments for COPY and ADD

The current implementation silently ignores the chmod option argument
for COPY and ADD if it is invalid, so this PR change to check the
validity of the argument.

I also found that if the value is 0o3777777777777(-1), it is ignored, and
otherwise all but the lower 12 bits are meaningless.

Signed-off-by: Mitsuru Kariya <[email protected]>

Author: kariya-mitsuru

frontend/dockerfile/dockerfile2llb/convert.go

@@ -1363,10 +1363,11 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error {
 	var mode *os.FileMode
 	if cfg.chmod != "" {
 		p, err := strconv.ParseUint(cfg.chmod, 8, 32)
-		if err == nil {
-			perm := os.FileMode(p)
-			mode = &perm
+		if err != nil || p > 0o7777 {
+			return errors.Errorf("invalid chmod parameter: '%v'. it should be octal string and between 0 and 07777", cfg.chmod)
 		}
+		perm := os.FileMode(p)
+		mode = &perm
 	}
 
 	if cfg.checksum != "" {

frontend/dockerfile/dockerfile_test.go

@@ -161,11 +161,13 @@ var allTests = integration.TestFuncs(
 	testCopySymlinks,
 	testCopyChown,
 	testCopyChmod,
+	testCopyInvalidChmod,
 	testCopyOverrideFiles,
 	testCopyVarSubstitution,
 	testCopyWildcards,
 	testCopyRelative,
 	testAddURLChmod,
+	testAddInvalidChmod,
 	testTarContext,
 	testTarContextExternalDockerfile,
 	testWorkdirUser,
@@ -3516,6 +3518,57 @@ COPY --from=base /out /
 	require.Equal(t, "0000\n", string(dt))
 }
 
+func testCopyInvalidChmod(t *testing.T, sb integration.Sandbox) {
+	integration.SkipOnPlatform(t, "windows")
+	f := getFrontend(t, sb)
+
+	dockerfile := []byte(`
+FROM scratch
+COPY --chmod=64a foo /
+`)
+
+	dir := integration.Tmpdir(
+		t,
+		fstest.CreateFile("Dockerfile", dockerfile, 0600),
+		fstest.CreateFile("foo", []byte(`foo-contents`), 0600),
+	)
+
+	c, err := client.New(sb.Context(), sb.Address())
+	require.NoError(t, err)
+	defer c.Close()
+
+	_, err = f.Solve(sb.Context(), c, client.SolveOpt{
+		LocalMounts: map[string]fsutil.FS{
+			dockerui.DefaultLocalNameDockerfile: dir,
+			dockerui.DefaultLocalNameContext:    dir,
+		},
+	}, nil)
+	require.ErrorContains(t, err, "invalid chmod parameter: '64a'. it should be octal string and between 0 and 07777")
+
+	dockerfile = []byte(`
+FROM scratch
+COPY --chmod=10000 foo /
+`)
+
+	dir = integration.Tmpdir(
+		t,
+		fstest.CreateFile("Dockerfile", dockerfile, 0600),
+		fstest.CreateFile("foo", []byte(`foo-contents`), 0600),
+	)
+
+	c, err = client.New(sb.Context(), sb.Address())
+	require.NoError(t, err)
+	defer c.Close()
+
+	_, err = f.Solve(sb.Context(), c, client.SolveOpt{
+		LocalMounts: map[string]fsutil.FS{
+			dockerui.DefaultLocalNameDockerfile: dir,
+			dockerui.DefaultLocalNameContext:    dir,
+		},
+	}, nil)
+	require.ErrorContains(t, err, "invalid chmod parameter: '10000'. it should be octal string and between 0 and 07777")
+}
+
 func testCopyOverrideFiles(t *testing.T, sb integration.Sandbox) {
 	integration.SkipOnPlatform(t, "windows")
 	f := getFrontend(t, sb)
@@ -3803,6 +3856,57 @@ COPY --from=build /dest /dest
 	require.Equal(t, []byte("0644\n0755\n0413\n"), dt)
 }
 
+func testAddInvalidChmod(t *testing.T, sb integration.Sandbox) {
+	integration.SkipOnPlatform(t, "windows")
+	f := getFrontend(t, sb)
+
+	dockerfile := []byte(`
+FROM scratch
+ADD --chmod=64a foo /
+`)
+
+	dir := integration.Tmpdir(
+		t,
+		fstest.CreateFile("Dockerfile", dockerfile, 0600),
+		fstest.CreateFile("foo", []byte(`foo-contents`), 0600),
+	)
+
+	c, err := client.New(sb.Context(), sb.Address())
+	require.NoError(t, err)
+	defer c.Close()
+
+	_, err = f.Solve(sb.Context(), c, client.SolveOpt{
+		LocalMounts: map[string]fsutil.FS{
+			dockerui.DefaultLocalNameDockerfile: dir,
+			dockerui.DefaultLocalNameContext:    dir,
+		},
+	}, nil)
+	require.ErrorContains(t, err, "invalid chmod parameter: '64a'. it should be octal string and between 0 and 07777")
+
+	dockerfile = []byte(`
+FROM scratch
+ADD --chmod=10000 foo /
+`)
+
+	dir = integration.Tmpdir(
+		t,
+		fstest.CreateFile("Dockerfile", dockerfile, 0600),
+		fstest.CreateFile("foo", []byte(`foo-contents`), 0600),
+	)
+
+	c, err = client.New(sb.Context(), sb.Address())
+	require.NoError(t, err)
+	defer c.Close()
+
+	_, err = f.Solve(sb.Context(), c, client.SolveOpt{
+		LocalMounts: map[string]fsutil.FS{
+			dockerui.DefaultLocalNameDockerfile: dir,
+			dockerui.DefaultLocalNameContext:    dir,
+		},
+	}, nil)
+	require.ErrorContains(t, err, "invalid chmod parameter: '10000'. it should be octal string and between 0 and 07777")
+}
+
 func testDockerfileFromGit(t *testing.T, sb integration.Sandbox) {
 	integration.SkipOnPlatform(t, "windows")
 	f := getFrontend(t, sb)