fix(download): harden filename sanitization and error handling
- commit
- 4a8a8379eab9136b1a51aa622271189ea4dba41c
- author
- bnema · · 2026-01-07 18:25 UTC
- parent
- 08eefbd
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +15 -3
M internal/infrastructure/webkit/download_handler.go +13 -1
| @@ -52,6 +52,7 @@ func (h *DownloadHandler) HandleDownload(ctx context.Context, download *webkit.D | |||
| 52 | 52 | // Ensure download directory exists. | |
| 53 | 53 | if err := os.MkdirAll(downloadPath, dirPerm); err != nil { | |
| 54 | 54 | log.Error().Err(err).Str("path", downloadPath).Msg("failed to create download directory") | |
| 55 | + | d.Cancel() | |
| 55 | 56 | return false | |
| 56 | 57 | } | |
| 57 | 58 | ||
| @@ -149,12 +150,23 @@ func (h *DownloadHandler) SetDownloadPath(path string) { | |||
| 149 | 150 | func extractFilename(dest string) string { | |
| 150 | 151 | // Remove file:// prefix if present. | |
| 151 | 152 | path := strings.TrimPrefix(dest, "file://") | |
| 152 | - | return filepath.Base(path) | |
| 153 | + | base := filepath.Base(path) | |
| 154 | + | ||
| 155 | + | // Handle edge cases consistently with sanitizeFilename. | |
| 156 | + | if base == "." || base == "" { | |
| 157 | + | return "download" | |
| 158 | + | } | |
| 159 | + | return base | |
| 153 | 160 | } | |
| 154 | 161 | ||
| 155 | 162 | // sanitizeFilename sanitizes a suggested filename to prevent path traversal attacks. | |
| 156 | 163 | // It extracts only the base name and handles edge cases like "." or "..". | |
| 157 | 164 | func sanitizeFilename(name string) string { | |
| 165 | + | // Normalize Windows-style separators to forward slashes. | |
| 166 | + | // filepath.Base only handles the OS-native separator, so on Linux | |
| 167 | + | // backslashes would not be treated as path separators. | |
| 168 | + | name = strings.ReplaceAll(name, "\\", "/") | |
| 169 | + | ||
| 158 | 170 | // Get only the base name (removes any directory components). | |
| 159 | 171 | clean := filepath.Base(name) | |
| 160 | 172 | ||
| @@ -52,6 +52,7 @@ func (h *DownloadHandler) HandleDownload(ctx context.Context, download *webkit.D | |||||
| 52 | // Ensure download directory exists. | 52 | // Ensure download directory exists. | ||
| 53 | if err := os.MkdirAll(downloadPath, dirPerm); err != nil { | 53 | if err := os.MkdirAll(downloadPath, dirPerm); err != nil { | ||
| 54 | log.Error().Err(err).Str("path", downloadPath).Msg("failed to create download directory") | 54 | log.Error().Err(err).Str("path", downloadPath).Msg("failed to create download directory") | ||
| 55 | + | d.Cancel() | |||
| 55 | return false | 56 | return false | ||
| 56 | } | 57 | } | ||
| 57 | 58 | ||||
| @@ -149,12 +150,23 @@ func (h *DownloadHandler) SetDownloadPath(path string) { | |||||
| 149 | func extractFilename(dest string) string { | 150 | func extractFilename(dest string) string { | ||
| 150 | // Remove file:// prefix if present. | 151 | // Remove file:// prefix if present. | ||
| 151 | path := strings.TrimPrefix(dest, "file://") | 152 | path := strings.TrimPrefix(dest, "file://") | ||
| 152 | - | return filepath.Base(path) | 153 | + | base := filepath.Base(path) |
| 154 | + | ||||
| 155 | + | // Handle edge cases consistently with sanitizeFilename. | |||
| 156 | + | if base == "." || base == "" { | |||
| 157 | + | return "download" | |||
| 158 | + | } | |||
| 159 | + | return base | |||
| 153 | } | 160 | } | ||
| 154 | 161 | ||||
| 155 | // sanitizeFilename sanitizes a suggested filename to prevent path traversal attacks. | 162 | // sanitizeFilename sanitizes a suggested filename to prevent path traversal attacks. | ||
| 156 | // It extracts only the base name and handles edge cases like "." or "..". | 163 | // It extracts only the base name and handles edge cases like "." or "..". | ||
| 157 | func sanitizeFilename(name string) string { | 164 | func sanitizeFilename(name string) string { | ||
| 165 | + | // Normalize Windows-style separators to forward slashes. | |||
| 166 | + | // filepath.Base only handles the OS-native separator, so on Linux | |||
| 167 | + | // backslashes would not be treated as path separators. | |||
| 168 | + | name = strings.ReplaceAll(name, "\\", "/") | |||
| 169 | + | ||||
| 158 | // Get only the base name (removes any directory components). | 170 | // Get only the base name (removes any directory components). | ||
| 159 | clean := filepath.Base(name) | 171 | clean := filepath.Base(name) | ||
| 160 | 172 | ||||
M internal/infrastructure/webkit/download_handler_test.go +2 -2
| @@ -65,7 +65,7 @@ func TestSanitizeFilename(t *testing.T) { | |||
| 65 | 65 | { | |
| 66 | 66 | name: "windows style path", | |
| 67 | 67 | input: "..\\..\\Windows\\System32\\config", | |
| 68 | - | expected: "..\\..\\Windows\\System32\\config", | |
| 68 | + | expected: "config", | |
| 69 | 69 | }, | |
| 70 | 70 | } | |
| 71 | 71 | ||
| @@ -106,7 +106,7 @@ func TestExtractFilename(t *testing.T) { | |||
| 106 | 106 | { | |
| 107 | 107 | name: "empty string", | |
| 108 | 108 | input: "", | |
| 109 | - | expected: ".", | |
| 109 | + | expected: "download", | |
| 110 | 110 | }, | |
| 111 | 111 | { | |
| 112 | 112 | name: "file URI with spaces", | |
| @@ -65,7 +65,7 @@ func TestSanitizeFilename(t *testing.T) { | |||||
| 65 | { | 65 | { | ||
| 66 | name: "windows style path", | 66 | name: "windows style path", | ||
| 67 | input: "..\\..\\Windows\\System32\\config", | 67 | input: "..\\..\\Windows\\System32\\config", | ||
| 68 | - | expected: "..\\..\\Windows\\System32\\config", | 68 | + | expected: "config", |
| 69 | }, | 69 | }, | ||
| 70 | } | 70 | } | ||
| 71 | 71 | ||||
| @@ -106,7 +106,7 @@ func TestExtractFilename(t *testing.T) { | |||||
| 106 | { | 106 | { | ||
| 107 | name: "empty string", | 107 | name: "empty string", | ||
| 108 | input: "", | 108 | input: "", | ||
| 109 | - | expected: ".", | 109 | + | expected: "download", |
| 110 | }, | 110 | }, | ||
| 111 | { | 111 | { | ||
| 112 | name: "file URI with spaces", | 112 | name: "file URI with spaces", | ||