Skip to content

Commit ee18834

Browse files
maximbiglermaximbigler
committed
fix: Path injection in deploy
1 parent e9aa029 commit ee18834

3 files changed

Lines changed: 266 additions & 5 deletions

File tree

‎backend/internal/agent/docker/deploy.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,10 +48,16 @@ func (c *Client) Deploy(ctx context.Context, req DeployRequest) error {
4848
return errors.New("compose file is empty")
4949
}
5050

51+
// Validate application name to prevent path traversal
52+
if err := utils.DoesNotLookLikeFilePath(req.ApplicationName); err != nil {
53+
return fmt.Errorf("invalid application name: %w", err)
54+
}
55+
5156
applicationDir := filepath.Join(c.deploymentsDir, req.ApplicationName)
5257
composePath := filepath.Join(applicationDir, composeFileName)
5358

54-
if err := utils.DoesNotLookLikeFilePath(composePath); err != nil {
59+
// Verify the final path stays within the deployments directory
60+
if err := utils.IsPathWithinBase(c.deploymentsDir, composePath); err != nil {
5561
return fmt.Errorf("invalid compose file path: %w", err)
5662
}
5763

‎backend/internal/shared/utils/fileUtils.go‎

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,24 @@ package utils
22

33
import (
44
"errors"
5+
"fmt"
56
"path/filepath"
67
"strings"
78
)
89

910
type FileUtils struct{}
1011

12+
// DoesNotLookLikeFilePath validates that a path is safe and doesn't contain path traversal sequences.
1113
func DoesNotLookLikeFilePath(name string) error {
1214
if strings.TrimSpace(name) == "" {
1315
return errors.New("file path cannot be empty")
1416
}
1517

18+
// Prevent path traversal attacks
19+
if strings.Contains(name, "..") {
20+
return errors.New("file path cannot contain '..'")
21+
}
22+
1623
// Check for invalid characters that shouldn't appear in file paths
1724
invalidChars := []string{"<", ">", ":", "\"", "|", "?", "*"}
1825
for _, char := range invalidChars {
@@ -21,10 +28,25 @@ func DoesNotLookLikeFilePath(name string) error {
2128
}
2229
}
2330

24-
// Check if path is clean and doesn't have suspicious patterns
25-
absPath := filepath.Clean(name)
26-
if absPath == "." || absPath == ".." {
27-
return errors.New("file path must be a valid path")
31+
return nil
32+
}
33+
34+
// IsPathWithinBase ensures that a file path stays within a base directory.
35+
// This prevents directory traversal attacks even with symlinks and relative paths.
36+
func IsPathWithinBase(basePath, filePath string) error {
37+
absBase, err := filepath.Abs(basePath)
38+
if err != nil {
39+
return fmt.Errorf("invalid base path: %w", err)
40+
}
41+
42+
absFile, err := filepath.Abs(filePath)
43+
if err != nil {
44+
return fmt.Errorf("invalid file path: %w", err)
45+
}
46+
47+
// Ensure the file path is within the base directory
48+
if !strings.HasPrefix(absFile, absBase) {
49+
return errors.New("path traversal detected: file path is outside base directory")
2850
}
2951

3052
return nil

‎backend/internal/shared/utils/fileUtils_test.go‎

Lines changed: 233 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,233 @@
1+
package utils
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"testing"
7+
)
8+
9+
func TestDoesNotLookLikeFilePath(t *testing.T) {
10+
tests := []struct {
11+
name string
12+
input string
13+
wantErr bool
14+
errMsg string
15+
}{
16+
{
17+
name: "valid simple path",
18+
input: "file.txt",
19+
wantErr: false,
20+
},
21+
{
22+
name: "valid nested path",
23+
input: "dir/subdir/file.txt",
24+
wantErr: false,
25+
},
26+
{
27+
name: "valid path with hyphens",
28+
input: "my-app-file.yaml",
29+
wantErr: false,
30+
},
31+
{
32+
name: "valid path with underscores",
33+
input: "my_app_file.yaml",
34+
wantErr: false,
35+
},
36+
{
37+
name: "empty string",
38+
input: "",
39+
wantErr: true,
40+
errMsg: "file path cannot be empty",
41+
},
42+
{
43+
name: "whitespace only",
44+
input: " ",
45+
wantErr: true,
46+
errMsg: "file path cannot be empty",
47+
},
48+
{
49+
name: "path with parent directory traversal",
50+
input: "../etc/passwd",
51+
wantErr: true,
52+
errMsg: "file path cannot contain '..'",
53+
},
54+
{
55+
name: "path with double dots in middle",
56+
input: "dir/../file.txt",
57+
wantErr: true,
58+
errMsg: "file path cannot contain '..'",
59+
},
60+
{
61+
name: "path with double dots at start",
62+
input: "../../etc/shadow",
63+
wantErr: true,
64+
errMsg: "file path cannot contain '..'",
65+
},
66+
{
67+
name: "path with less than character",
68+
input: "file<name>.txt",
69+
wantErr: true,
70+
errMsg: "file path contains invalid character: <",
71+
},
72+
{
73+
name: "path with greater than character",
74+
input: "file>name.txt",
75+
wantErr: true,
76+
errMsg: "file path contains invalid character: >",
77+
},
78+
{
79+
name: "path with colon",
80+
input: "file:name.txt",
81+
wantErr: true,
82+
errMsg: "file path contains invalid character: :",
83+
},
84+
{
85+
name: "path with double quote",
86+
input: "file\"name\".txt",
87+
wantErr: true,
88+
errMsg: "file path contains invalid character: \"",
89+
},
90+
{
91+
name: "path with pipe",
92+
input: "file|name.txt",
93+
wantErr: true,
94+
errMsg: "file path contains invalid character: |",
95+
},
96+
{
97+
name: "path with question mark",
98+
input: "file?name.txt",
99+
wantErr: true,
100+
errMsg: "file path contains invalid character: ?",
101+
},
102+
{
103+
name: "path with asterisk",
104+
input: "file*name.txt",
105+
wantErr: true,
106+
errMsg: "file path contains invalid character: *",
107+
},
108+
}
109+
110+
for _, tt := range tests {
111+
t.Run(tt.name, func(t *testing.T) {
112+
err := DoesNotLookLikeFilePath(tt.input)
113+
if (err != nil) != tt.wantErr {
114+
t.Errorf("DoesNotLookLikeFilePath() error = %v, wantErr %v", err, tt.wantErr)
115+
}
116+
if tt.wantErr && tt.errMsg != "" && err.Error() != tt.errMsg {
117+
t.Errorf("DoesNotLookLikeFilePath() error message = %q, want %q", err.Error(), tt.errMsg)
118+
}
119+
})
120+
}
121+
}
122+
123+
func TestIsPathWithinBase(t *testing.T) {
124+
// Create temporary directories for testing
125+
tmpDir := t.TempDir()
126+
baseDir := filepath.Join(tmpDir, "base")
127+
if err := os.MkdirAll(baseDir, 0o755); err != nil {
128+
t.Fatalf("failed to create base directory: %v", err)
129+
}
130+
131+
tests := []struct {
132+
name string
133+
baseDir string
134+
filePath string
135+
wantErr bool
136+
errMsg string
137+
}{
138+
{
139+
name: "file directly in base directory",
140+
baseDir: baseDir,
141+
filePath: filepath.Join(baseDir, "file.txt"),
142+
wantErr: false,
143+
},
144+
{
145+
name: "file in subdirectory of base",
146+
baseDir: baseDir,
147+
filePath: filepath.Join(baseDir, "subdir", "file.txt"),
148+
wantErr: false,
149+
},
150+
{
151+
name: "file deeply nested in base",
152+
baseDir: baseDir,
153+
filePath: filepath.Join(baseDir, "a", "b", "c", "file.txt"),
154+
wantErr: false,
155+
},
156+
{
157+
name: "relative path within base (with ./)",
158+
baseDir: baseDir,
159+
filePath: filepath.Join(baseDir, ".", "file.txt"),
160+
wantErr: false,
161+
},
162+
{
163+
name: "file outside base directory",
164+
baseDir: baseDir,
165+
filePath: filepath.Join(tmpDir, "other", "file.txt"),
166+
wantErr: true,
167+
errMsg: "path traversal detected: file path is outside base directory",
168+
},
169+
{
170+
name: "file in parent directory of base",
171+
baseDir: baseDir,
172+
filePath: filepath.Join(tmpDir, "file.txt"),
173+
wantErr: true,
174+
errMsg: "path traversal detected: file path is outside base directory",
175+
},
176+
{
177+
name: "relative path traversing out of base",
178+
baseDir: baseDir,
179+
filePath: filepath.Join(baseDir, "subdir", "..", "..", "file.txt"),
180+
wantErr: true,
181+
errMsg: "path traversal detected: file path is outside base directory",
182+
},
183+
{
184+
name: "base directory itself",
185+
baseDir: baseDir,
186+
filePath: baseDir,
187+
wantErr: false,
188+
},
189+
}
190+
191+
for _, tt := range tests {
192+
t.Run(tt.name, func(t *testing.T) {
193+
err := IsPathWithinBase(tt.baseDir, tt.filePath)
194+
if (err != nil) != tt.wantErr {
195+
t.Errorf("IsPathWithinBase() error = %v, wantErr %v", err, tt.wantErr)
196+
}
197+
if tt.wantErr && tt.errMsg != "" && err.Error() != tt.errMsg {
198+
t.Errorf("IsPathWithinBase() error message = %q, want %q", err.Error(), tt.errMsg)
199+
}
200+
})
201+
}
202+
}
203+
204+
func TestIsPathWithinBaseInvalidInput(t *testing.T) {
205+
tests := []struct {
206+
name string
207+
baseDir string
208+
filePath string
209+
wantErr bool
210+
}{
211+
{
212+
name: "invalid base directory path",
213+
baseDir: "/nonexistent/base/path/that/is/invalid\x00",
214+
filePath: "/some/file.txt",
215+
wantErr: true,
216+
},
217+
{
218+
name: "invalid file path",
219+
baseDir: "/tmp",
220+
filePath: "/some/file/path\x00",
221+
wantErr: true,
222+
},
223+
}
224+
225+
for _, tt := range tests {
226+
t.Run(tt.name, func(t *testing.T) {
227+
err := IsPathWithinBase(tt.baseDir, tt.filePath)
228+
if (err != nil) != tt.wantErr {
229+
t.Errorf("IsPathWithinBase() error = %v, wantErr %v", err, tt.wantErr)
230+
}
231+
})
232+
}
233+
}

0 commit comments

Comments
 (0)