Skip to content

Commit d859abf

Browse files
authored
security: validate batch spec mount name (#1284)
1 parent 78c41d4 commit d859abf

File tree

2 files changed

+29
-4
lines changed

2 files changed

+29
-4
lines changed

internal/batches/service/service_test.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -594,6 +594,26 @@ changesetTemplate:
594594
`,
595595
expectedErr: errors.New("handling mount: step 1 mount path is not in the same directory or subdirectory as the batch spec"),
596596
},
597+
{
598+
name: "files target path with comma",
599+
batchSpecDir: tempDir,
600+
rawSpec: `
601+
name: test-spec
602+
description: A test spec
603+
steps:
604+
- run: echo "hello"
605+
container: alpine:3
606+
files:
607+
"/tmp/x,source=/var/run/docker.sock,target=/var/run/docker.sock": "IGNORED"
608+
changesetTemplate:
609+
title: Test Files
610+
body: Test files target path with comma
611+
branch: test
612+
commit:
613+
message: Test
614+
`,
615+
expectedErr: errors.New("parsing batch spec: step 1 files target path contains invalid characters"),
616+
},
597617
{
598618
name: "mount path dot-dot traversal",
599619
batchSpecDir: tempDir,

lib/batches/batch_spec.go

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -169,19 +169,24 @@ func parseBatchSpec(schema string, data []byte) (*BatchSpec, error) {
169169

170170
for i, step := range spec.Steps {
171171
for _, mount := range step.Mount {
172-
if strings.Contains(mount.Path, invalidMountCharacters) {
172+
if strings.ContainsAny(mount.Path, invalidMountCharacters) {
173173
errs = errors.Append(errs, NewValidationError(errors.Newf("step %d mount path contains invalid characters", i+1)))
174174
}
175-
if strings.Contains(mount.Mountpoint, invalidMountCharacters) {
175+
if strings.ContainsAny(mount.Mountpoint, invalidMountCharacters) {
176176
errs = errors.Append(errs, NewValidationError(errors.Newf("step %d mount mountpoint contains invalid characters", i+1)))
177177
}
178178
}
179+
for name := range step.Files {
180+
if strings.ContainsAny(name, invalidMountCharacters) {
181+
errs = errors.Append(errs, NewValidationError(errors.Newf("step %d files target path contains invalid characters", i+1)))
182+
}
183+
}
179184
}
180185

181186
return &spec, errs
182187
}
183-
184-
const invalidMountCharacters = ","
188+
// docker uses Golang's `encoding/csv` library to parse arguments passed to `--mount`
189+
const invalidMountCharacters = ",\"\n\r"
185190

186191
func (on *OnQueryOrRepository) String() string {
187192
if on.RepositoriesMatchingQuery != "" {

0 commit comments

Comments
 (0)