Skip to content

Commit 89b653f

Browse files
mklos-kwmklos-kw
committed
fix: [OCISDEV-175] server path validation
1 parent dff2267 commit 89b653f

File tree

2 files changed

+320
-0
lines changed

2 files changed

+320
-0
lines changed

‎services/web/pkg/assets/server.go‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,15 @@ import (
66
"io/fs"
77
"mime"
88
"net/http"
9+
"net/url"
910
"path"
1011
"path/filepath"
12+
"regexp"
13+
"strings"
14+
"unicode/utf8"
1115

1216
"golang.org/x/net/html"
17+
"golang.org/x/text/unicode/norm"
1318
)
1419

1520
type fileServer struct {
@@ -21,7 +26,58 @@ func FileServer(fsys fs.FS) http.Handler {
2126
return &fileServer{http.FS(fsys)}
2227
}
2328

29+
// IsSafePath validates if a path is safe from path traversal attacks
30+
func IsSafePath(p string) bool {
31+
var dangerousRunes = []rune{
32+
'\uFF0F', // fullwidth slash ／
33+
'\u2215', // division slash ∕
34+
'\u29F8', // big solidus ⧸
35+
'\u2044', // fraction slash ⁄
36+
'\\', // backslash
37+
'\uFF3C', // fullwidth backslash ＼
38+
'\uFE68', // small reverse solidus ﹨
39+
}
40+
41+
// Recursive URL decode to prevent bypass via %252e%252e (double encoding)
42+
path := p
43+
for {
44+
decoded, err := url.QueryUnescape(path)
45+
if err != nil || decoded == path {
46+
break
47+
}
48+
path = decoded
49+
}
50+
51+
// Reject null byte injection
52+
if strings.Contains(path, "\x00") {
53+
return false
54+
}
55+
56+
// Reject invalid UTF-8
57+
if !utf8.ValidString(path) {
58+
return false
59+
}
60+
61+
// Reject dangerous runes
62+
normalized := norm.NFC.String(path)
63+
for _, r := range normalized {
64+
for _, bad := range dangerousRunes {
65+
if r == bad {
66+
return false
67+
}
68+
}
69+
}
70+
71+
// match one or more /./ and /../
72+
return !regexp.MustCompile(`(?:[\/\\]\.+[\/\\])+`).MatchString(path)
73+
}
74+
2475
func (f *fileServer) ServeHTTP(w http.ResponseWriter, r *http.Request) {
76+
if !IsSafePath(r.URL.Path) {
77+
http.NotFound(w, r)
78+
return
79+
}
80+
2581
uPath := path.Clean(path.Join("/", r.URL.Path))
2682
r.URL.Path = uPath
2783

‎services/web/pkg/assets/server_test.go‎

Lines changed: 264 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,5 +119,269 @@ func TestFileServer(t *testing.T) {
119119

120120
})
121121
}
122+
}
123+
124+
func TestIsSafePath(t *testing.T) {
125+
cases := []struct {
126+
expected bool
127+
path string
128+
name string
129+
}{
130+
// API endpoints that should be allowed
131+
{expected: true, path: "/graph/v1.0/shares", name: "graph api endpoint"},
132+
{expected: true, path: "/graph/v1.0/shares/123", name: "graph api with path params"},
133+
{expected: true, path: "/graph/v1.0/users/me/drive/items/123:/testfile.txt:/permissions", name: "graph api with complex path"},
134+
{expected: true, path: "/ocs/v1.php/cloud/users", name: "ocs api endpoint"},
135+
{expected: true, path: "/ocs/v2.php/apps/files_sharing/api/v1/shares", name: "ocs api with subpath"},
136+
{expected: true, path: "/remote.php/webdav/testfile.txt", name: "remote.php endpoint"},
137+
{expected: true, path: "/remote.php/dav/files/alice/testfile.txt", name: "remote.php with dav"},
138+
139+
// File serving paths that should be validated
140+
{expected: true, path: "/index.html", name: "simple file path"},
141+
{expected: true, path: "/assets/css/style.css", name: "subdirectory file"},
142+
{expected: true, path: "/js/vendor/jquery.min.js", name: "nested directory"},
143+
144+
// Path traversal attempts that should be blocked
145+
{expected: false, path: "/assets/../secret.txt", name: "traversal with dots"},
146+
{expected: false, path: "/assets/%2e%2e/secret.txt", name: "traversal with encoded dots"},
147+
{expected: false, path: "/assets/./secret.txt", name: "current directory"},
148+
{expected: false, path: "/assets/%2e/secret.txt", name: "encoded current directory"},
149+
{expected: false, path: "/assets/css/../../secret.txt", name: "double traversal"},
150+
{expected: false, path: "/assets/../css/../../secret.txt", name: "mixed traversal"},
151+
152+
// Edge cases
153+
{expected: true, path: "", name: "empty path"},
154+
{expected: true, path: "/", name: "root path"},
155+
{expected: true, path: "//assets/file.txt", name: "double slash"},
156+
{expected: true, path: "/assets/", name: "trailing slash"},
157+
158+
// Additional test cases based on Go blog post about os.Root
159+
// URL-encoded traversal attempts
160+
{expected: false, path: "/assets/%2e%2e%2fsecret.txt", name: "URL encoded traversal"},
161+
{expected: false, path: "/assets/%2e%2e%5csecret.txt", name: "URL encoded backslash traversal"},
162+
{expected: false, path: "/assets/%252e%252e/secret.txt", name: "double URL encoded dots"},
163+
{expected: false, path: "/assets/%252e%252e%252fsecret.txt", name: "double URL encoded traversal"},
164+
165+
// Unicode normalization attacks
166+
{expected: false, path: "/assets/..%c0%afsecret.txt", name: "UTF-8 encoded traversal"},
167+
{expected: false, path: "/assets/..%ef%bc%8fsecret.txt", name: "fullwidth slash traversal"},
168+
{expected: false, path: "/assets/..%c1%9csecret.txt", name: "UTF-8 encoded backslash"},
169+
170+
// Multiple encoding layers
171+
{expected: false, path: "/assets/%252e%252e%252fsecret.txt", name: "double percent encoding"},
172+
{expected: false, path: "/assets/%252e%252e%255csecret.txt", name: "double percent encoding backslash"},
173+
174+
// Null byte injection attempts
175+
{expected: false, path: "/assets/..%00/secret.txt", name: "null byte injection"},
176+
{expected: false, path: "/assets/%00../secret.txt", name: "null byte prefix"},
177+
178+
// Mixed case and encoding
179+
{expected: false, path: "/assets/..%2Fsecret.txt", name: "mixed case URL encoding"},
180+
{expected: false, path: "/assets/..%2fsecret.txt", name: "lowercase URL encoding"},
181+
{expected: false, path: "/assets/..%5Csecret.txt", name: "uppercase backslash encoding"},
182+
183+
// Path normalization edge cases
184+
{expected: false, path: "/assets/././../secret.txt", name: "redundant current dir traversal"},
185+
{expected: false, path: "/assets/.././secret.txt", name: "mixed traversal current dir"},
186+
{expected: false, path: "/assets/.../secret.txt", name: "triple dots"},
187+
{expected: false, path: "/assets/..../secret.txt", name: "quadruple dots"},
188+
189+
// Paths that should be allowed (no traversal)
190+
{expected: true, path: "/assets/dots.in.filename.txt", name: "dots in filename"},
191+
{expected: true, path: "/assets/file..txt", name: "dots at end of filename"},
192+
{expected: true, path: "/assets/.hidden", name: "hidden file"},
193+
{expected: true, path: "/assets/..hidden", name: "file starting with dots"},
194+
{expected: true, path: "/assets/...hidden", name: "file starting with triple dots"},
195+
{expected: true, path: "/assets/normal/path/with/dots.txt", name: "normal path with dots"},
196+
{expected: true, path: "/assets/path/with/encoded%20spaces.txt", name: "URL encoded spaces"},
197+
{expected: true, path: "/assets/path/with/unicode%c3%a9.txt", name: "URL encoded unicode"},
198+
199+
// API paths that might contain dots but should be allowed
200+
{expected: true, path: "/graph/v1.0/users/me/drive/items/123:/file..txt:/permissions", name: "graph api with dots in filename"},
201+
{expected: true, path: "/ocs/v1.php/cloud/users/..user", name: "ocs api with dots in username"},
202+
{expected: true, path: "/remote.php/dav/files/alice/.config", name: "remote.php with hidden file"},
203+
204+
// e2e
205+
// - navigating to "https://ocis-server:9200/files/spaces/personal/alice/parent/folder%252Fwith%252FSlashes?fileId=048eb01c-483c-4a9e-a3ac-17345d767500%24137d4fd3-afc7-4059-924b-834b0622f8c9%2158c2219f-2b84-444e-b814-8e077d4c496e&items-per-page=100&files-spaces-generic-view-mode=resource-table&tiles-size=2", waiting until "load"
206+
{expected: true, path: "files/spaces/personal/alice/parent/folder%252Fwith%252FSlashes", name: "e2e"},
207+
}
208+
209+
for _, tc := range cases {
210+
t.Run(tc.name, func(t *testing.T) {
211+
result := IsSafePath(tc.path)
212+
if result != tc.expected {
213+
t.Errorf("isSafePath(%q) = %v, want %v", tc.path, result, tc.expected)
214+
}
215+
})
216+
}
217+
}
218+
219+
// TestPathTraversalVulnerabilities tests various path traversal attack vectors
220+
// mentioned in the Go blog post about os.Root https://go.dev/blog/osroot
221+
func TestPathTraversalVulnerabilities(t *testing.T) {
222+
g := gomega.NewWithT(t)
223+
224+
// Setup filesystem with sensitive files outside intended directory
225+
fsys := fstest.MapFS{
226+
"public": &fstest.MapFile{
227+
Mode: fs.ModeDir,
228+
},
229+
"public/legitimate.txt": &fstest.MapFile{
230+
Data: []byte("legitimate content"),
231+
},
232+
"secret.txt": &fstest.MapFile{
233+
Data: []byte("super-secret-content"),
234+
},
235+
"etc/passwd": &fstest.MapFile{
236+
Data: []byte("root:x:0:0:root:/root:/bin/bash"),
237+
},
238+
"config/database.yml": &fstest.MapFile{
239+
Data: []byte("database_password: secret123"),
240+
},
241+
}
242+
243+
testCases := []struct {
244+
name string
245+
path string
246+
isBlockExpected bool
247+
description string
248+
}{
249+
// Basic traversal attempts
250+
{
251+
name: "basic_traversal",
252+
path: "/public/../secret.txt",
253+
isBlockExpected: true,
254+
description: "Basic directory traversal with ..",
255+
},
256+
{
257+
name: "double_traversal",
258+
path: "/public/subdir/../../secret.txt",
259+
isBlockExpected: true,
260+
description: "Double directory traversal",
261+
},
262+
{
263+
name: "mixed_traversal",
264+
path: "/public/../config/../secret.txt",
265+
isBlockExpected: true,
266+
description: "Mixed traversal with current directory",
267+
},
268+
269+
// URL-encoded traversal attempts
270+
{
271+
name: "url_encoded_traversal",
272+
path: "/public/%2e%2e/secret.txt",
273+
isBlockExpected: true,
274+
description: "URL-encoded dots",
275+
},
276+
{
277+
name: "url_encoded_slash",
278+
path: "/public/%2e%2e%2fsecret.txt",
279+
isBlockExpected: true,
280+
description: "URL-encoded dots and slash",
281+
},
282+
{
283+
name: "double_encoded",
284+
path: "/public/%252e%252e/secret.txt",
285+
isBlockExpected: true,
286+
description: "Double URL-encoded dots",
287+
},
288+
289+
// Unicode normalization attacks
290+
{
291+
name: "utf8_encoded_slash",
292+
path: "/public/..%c0%afsecret.txt",
293+
isBlockExpected: true,
294+
description: "UTF-8 encoded slash",
295+
},
296+
{
297+
name: "fullwidth_slash",
298+
path: "/public/..%ef%bc%8fsecret.txt",
299+
isBlockExpected: true,
300+
description: "Fullwidth slash",
301+
},
302+
303+
// Windows-specific attacks
304+
{
305+
name: "windows_backslash",
306+
path: "/public/..\\secret.txt",
307+
isBlockExpected: true,
308+
description: "Windows backslash traversal",
309+
},
310+
{
311+
name: "windows_encoded_backslash",
312+
path: "/public/%2e%2e%5csecret.txt",
313+
isBlockExpected: true,
314+
description: "URL-encoded Windows backslash",
315+
},
316+
317+
// Null byte injection
318+
{
319+
name: "null_byte_injection",
320+
path: "/public/..%00/secret.txt",
321+
isBlockExpected: true,
322+
description: "Null byte injection",
323+
},
324+
325+
// Path normalization edge cases
326+
{
327+
name: "redundant_current_dir",
328+
path: "/public/././../secret.txt",
329+
isBlockExpected: true,
330+
description: "Redundant current directory references",
331+
},
332+
{
333+
name: "triple_dots",
334+
path: "/public/.../secret.txt",
335+
isBlockExpected: true,
336+
description: "Triple dots (should be treated as traversal)",
337+
},
338+
339+
// Legitimate paths that should be allowed
340+
{
341+
name: "legitimate_file",
342+
path: "/public/legitimate.txt",
343+
isBlockExpected: false,
344+
description: "Legitimate file access",
345+
},
346+
{
347+
name: "dots_in_filename",
348+
path: "/public/file..txt",
349+
isBlockExpected: true,
350+
description: "Dots in filename (not traversal)",
351+
},
352+
{
353+
name: "hidden_file",
354+
path: "/public/.hidden",
355+
isBlockExpected: true,
356+
description: "Hidden file (not traversal)",
357+
},
358+
}
359+
360+
for _, tc := range testCases {
361+
t.Run(tc.name, func(t *testing.T) {
362+
req := httptest.NewRequest("GET", tc.path, nil)
363+
w := httptest.NewRecorder()
364+
365+
FileServer(fsys).ServeHTTP(w, req)
366+
res := w.Result()
367+
defer res.Body.Close()
368+
369+
body, err := io.ReadAll(res.Body)
370+
g.Expect(err).ToNot(gomega.HaveOccurred())
122371

372+
if tc.isBlockExpected {
373+
// Should be blocked - return 404 and not contain sensitive data
374+
g.Expect(res.StatusCode).To(gomega.Equal(http.StatusNotFound))
375+
g.Expect(string(body)).ToNot(gomega.ContainSubstring("super-secret"))
376+
g.Expect(string(body)).ToNot(gomega.ContainSubstring("database_password"))
377+
g.Expect(string(body)).ToNot(gomega.ContainSubstring("root:x:0:0"))
378+
} else {
379+
// Should be allowed - return 200 and contain expected content
380+
g.Expect(res.StatusCode).To(gomega.Equal(http.StatusOK))
381+
if tc.path == "/public/legitimate.txt" {
382+
g.Expect(string(body)).To(gomega.ContainSubstring("legitimate content"))
383+
}
384+
}
385+
})
386+
}
123387
}

0 commit comments

Comments
 (0)