diff --git a/services/repository/files/pathutils.go b/services/repository/files/pathutils.go index d290fda550..b327fbe694 100644 --- a/services/repository/files/pathutils.go +++ b/services/repository/files/pathutils.go @@ -9,12 +9,19 @@ import ( "strings" ) -var fileNameComponentSanitizeRegexp = regexp.MustCompile(`(?i)\.\.|[<>:\"/\\|?*\x{0000}-\x{001F}]|^(con|prn|aux|nul|com\d|lpt\d)$`) +// var fileNameComponentSanitizeRegexp = regexp.MustCompile(`(?i)\.\.|[<>:\"/\\|?*\x{0000}-\x{001F}]|^(con|prn|aux|nul|com\d|lpt\d)$`) +var fileNameComponentSanitizeRegexp = regexp.MustCompile(`(?i)[<>:\"/\\|?*\x{0000}-\x{001F}]|^(con|prn|aux|nul|com\d|lpt\d)$`) // SanitizePath cleans and validates a file path func SanitizePath(inputPath string) (string, error) { // Normalize path separators s := strings.ReplaceAll(inputPath, "\\", "/") + + // We don't want a / or \\ as the beginning of a path + if strings.HasPrefix(inputPath, "/") { + return "", fmt.Errorf("path starts with / : %s", inputPath) + } + // Clean the path s = path.Clean(s) // Split the path components @@ -22,6 +29,15 @@ func SanitizePath(inputPath string) (string, error) { // Sanitize each path component var sanitizedComponents []string for _, component := range pathComponents { + // There is no reason why there should be a path segment with .. + if component == ".." { + return "", fmt.Errorf("path contains directory traversal: %s", s) + } + // There is no reason why there should be a path segment with . + if component == "." { + return "", fmt.Errorf("path contains directory traversal: %s", s) + } + // Trim whitespace and apply regex sanitization sanitizedComponent := strings.TrimSpace(fileNameComponentSanitizeRegexp.ReplaceAllString(component, "_")) diff --git a/tests/e2e/repo-files.test.e2e.ts b/tests/e2e/repo-files.test.e2e.ts index 26b0d4c8a9..f4c1d713cd 100644 --- a/tests/e2e/repo-files.test.e2e.ts +++ b/tests/e2e/repo-files.test.e2e.ts @@ -42,7 +42,7 @@ test('drag and drop upload normal and special characters', async ({page}) => { await dropzone.dispatchEvent('drop', {dataTransfer: dataTransferA}); await page.getByText('new branch').click(); - await save_visual(page); + await page.getByRole('textbox', {name: 'Name the new branch for this'}).fill(testID); await page.getByRole('button', {name: 'Propose file change'}).click(); @@ -51,8 +51,9 @@ test('drag and drop upload normal and special characters', async ({page}) => { await expect(page.getByRole('link', {name: 'double/nested/file.txt'})).toBeVisible(); await expect(page.getByRole('link', {name: 'special/äüöÄÜÖß.txt'})).toBeVisible(); await expect(page.getByRole('link', {name: 'special/Ʉ₦ł₵ØĐɆ.txt'})).toBeVisible(); - await expect(page.getByRole('link', {name: 'root_file.txt'})).toBeVisible(); - await save_visual(page); + // Since this is a file in root, there two links with the same label + // we take the on in #diff-file-tree + await expect(page.locator('#diff-file-tree').getByRole('link', {name: 'root_file.txt'})).toBeVisible(); }); test('drag and drop upload strange paths and spaces', async ({page}) => { @@ -66,26 +67,79 @@ test('drag and drop upload strange paths and spaces', async ({page}) => { const dataTransferA = await page.evaluateHandle(() => { const dt = new DataTransfer(); // add items in different folders - dt.items.add(new File(['1'], '../../dots.txt', {type: 'text/plain'})); - dt.items.add(new File(['2'], 'special/../../dots_vanish.txt', {type: 'text/plain'})); - dt.items.add(new File(['3'], '\\windows\\windows_slash.txt', {type: 'text/plain'})); - dt.items.add(new File(['4'], '/special/badfirstslash.txt', {type: 'text/plain'})); - dt.items.add(new File(['5'], 'special/S P A C E !.txt', {type: 'text/plain'})); + dt.items.add(new File(['1'], '..dots.txt', {type: 'text/plain'})); + dt.items.add(new File(['2'], '.dots.vanish.txt', {type: 'text/plain'})); + dt.items.add(new File(['3'], 'special/S P A C E !.txt', {type: 'text/plain'})); return dt; }); // and drop them to the upload area await dropzone.dispatchEvent('drop', {dataTransfer: dataTransferA}); await page.getByText('new branch').click(); - await save_visual(page); + await page.getByRole('textbox', {name: 'Name the new branch for this'}).fill(testID); await page.getByRole('button', {name: 'Propose file change'}).click(); // check that nested file structure is preserved - await expect(page.getByRole('link', {name: 'windows/windows_slash.txt'})).toBeVisible(); - await expect(page.getByRole('link', {name: '_/dots_vanish.txt'})).toBeVisible(); - await expect(page.getByRole('link', {name: 'special/badfirstslash.txt'})).toBeVisible(); + // Since this is a file in root, there two links with the same label + // we take the on in #diff-file-tree + await expect(page.locator('#diff-file-tree').getByRole('link', {name: '.dots.vanish.txt'})).toBeVisible(); await expect(page.getByRole('link', {name: 'special/S P A C E !.txt'})).toBeVisible(); - await expect(page.getByRole('link', {name: '_/_/dots.txt'})).toBeVisible(); + // Since this is a file in root, there two links with the same label + // we take the on in #diff-file-tree + await expect(page.locator('#diff-file-tree').getByRole('link', {name: '..dots.txt'})).toBeVisible(); +}); + +test('drag and drop upload broken path slash in front', async ({page}) => { + const response = await page.goto(`/user2/file-uploads/_upload/main/`); + expect(response?.status()).toBe(200); // Status OK + + const testID = dynamic_id(); + const dropzone = page.getByRole('button', {name: 'Drop files or click here to upload.'}); + + // create the virtual files + const dataTransferA = await page.evaluateHandle(() => { + const dt = new DataTransfer(); + // add items in different folders + dt.items.add(new File(['1'], '/special/badfirstslash.txt', {type: 'text/plain'})); + return dt; + }); + // and drop them to the upload area + await dropzone.dispatchEvent('drop', {dataTransfer: dataTransferA}); + + await page.getByText('new branch').click(); + + await page.getByRole('textbox', {name: 'Name the new branch for this'}).fill(testID); + await page.getByRole('button', {name: 'Propose file change'}).click(); + + await expect(page.getByText('Failed to upload files to')).toBeVisible(); + + await save_visual(page); +}); + +test('drag and drop upload broken path with traversal', async ({page}) => { + const response = await page.goto(`/user2/file-uploads/_upload/main/`); + expect(response?.status()).toBe(200); // Status OK + + const testID = dynamic_id(); + const dropzone = page.getByRole('button', {name: 'Drop files or click here to upload.'}); + + // create the virtual files + const dataTransferA = await page.evaluateHandle(() => { + const dt = new DataTransfer(); + // add items in different folders + dt.items.add(new File(['1'], '../baddots.txt', {type: 'text/plain'})); + return dt; + }); + // and drop them to the upload area + await dropzone.dispatchEvent('drop', {dataTransfer: dataTransferA}); + + await page.getByText('new branch').click(); + + await page.getByRole('textbox', {name: 'Name the new branch for this'}).fill(testID); + await page.getByRole('button', {name: 'Propose file change'}).click(); + + await expect(page.getByText('Failed to upload files to')).toBeVisible(); + await save_visual(page); });