chore: add url validation #103
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| field, err := writer.CreatePart(val.Header) | ||
| if err != nil { | ||
| ErrorLogger.Printf("Failed to create part for file %s: %v", sanitizeLogInput(val.Filename), err) | ||
| ErrorLogger.Printf("Failed to create part for file %s: %v", val.Filename, err) |
Check failure
Code scanning / CodeQL
Log entries created from user input High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months ago
To fix this problem, all occurrences where user-supplied file names are logged should be sanitized before being passed to the logger. The best way to do this without changing existing functionality is to apply the existing sanitizeLogInput utility function to val.Filename in each affected logging statement.
Specifically, edit the relevant lines:
- Before logging
val.FilenameinErrorLogger.Printf, pass it through thesanitizeLogInputfunction.
This change should be applied to all logging lines in this code block that output val.Filename (lines 620, 626, 636, and 641).
No additional imports or definitions are required, as both the logger and the sanitizer are already present.
| @@ -617,13 +617,13 @@ | ||
| for _, val := range request.MultipartForm.File[fileKey] { | ||
| f, err := val.Open() | ||
| if err != nil { | ||
| ErrorLogger.Printf("Failed to open file %s: %v", val.Filename, err) | ||
| ErrorLogger.Printf("Failed to open file %s: %v", sanitizeLogInput(val.Filename), err) | ||
| continue | ||
| } | ||
|
|
||
| field, err := writer.CreatePart(val.Header) | ||
| if err != nil { | ||
| ErrorLogger.Printf("Failed to create part for file %s: %v", val.Filename, err) | ||
| ErrorLogger.Printf("Failed to create part for file %s: %v", sanitizeLogInput(val.Filename), err) | ||
| err = f.Close() | ||
| if err != nil { | ||
| ErrorLogger.Printf("Failed to close file: %v", err) | ||
| @@ -633,12 +628,12 @@ | ||
|
|
||
| _, err = io.Copy(field, f) | ||
| if err != nil { | ||
| ErrorLogger.Printf("Failed to copy file %s: %v", val.Filename, err) | ||
| ErrorLogger.Printf("Failed to copy file %s: %v", sanitizeLogInput(val.Filename), err) | ||
| } | ||
|
|
||
| err = f.Close() | ||
| if err != nil { | ||
| ErrorLogger.Printf("Failed to close file %s: %v", val.Filename, err) | ||
| ErrorLogger.Printf("Failed to close file %s: %v", sanitizeLogInput(val.Filename), err) | ||
| } | ||
| } | ||
| } |
| _, err = io.Copy(field, f) | ||
| if err != nil { | ||
| ErrorLogger.Printf("Failed to copy file %s: %v", sanitizeLogInput(val.Filename), err) | ||
| ErrorLogger.Printf("Failed to copy file %s: %v", val.Filename, err) |
Check failure
Code scanning / CodeQL
Log entries created from user input High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months ago
To fix this issue, we should sanitize any user-supplied data before writing it to logs. In this context, before logging val.Filename (which is derived from potentially untrusted user input), we should remove any newlines (\n), carriage returns (\r), and potentially other problematic characters. This ensures that a malicious user cannot inject log entries or otherwise tamper with the format of the log by submitting special characters in their filename.
The best way to do this, consistent with the rest of the codebase, is to use the provided sanitizeLogInput helper function (lines 23-28) before including any user-controlled value in a log statement. This simply requires wrapping each use of val.Filename in log statements (on lines 620, 626, 636, 641) with sanitizeLogInput().
No additional imports or utility methods are needed, as the helper function already exists and is in scope.
The only required code change is to replace any direct use of val.Filename in log calls in this region with sanitizeLogInput(val.Filename).
| @@ -617,13 +617,13 @@ | ||
| for _, val := range request.MultipartForm.File[fileKey] { | ||
| f, err := val.Open() | ||
| if err != nil { | ||
| ErrorLogger.Printf("Failed to open file %s: %v", val.Filename, err) | ||
| ErrorLogger.Printf("Failed to open file %s: %v", sanitizeLogInput(val.Filename), err) | ||
| continue | ||
| } | ||
|
|
||
| field, err := writer.CreatePart(val.Header) | ||
| if err != nil { | ||
| ErrorLogger.Printf("Failed to create part for file %s: %v", val.Filename, err) | ||
| ErrorLogger.Printf("Failed to create part for file %s: %v", sanitizeLogInput(val.Filename), err) | ||
| err = f.Close() | ||
| if err != nil { | ||
| ErrorLogger.Printf("Failed to close file: %v", err) | ||
| @@ -633,12 +628,12 @@ | ||
|
|
||
| _, err = io.Copy(field, f) | ||
| if err != nil { | ||
| ErrorLogger.Printf("Failed to copy file %s: %v", val.Filename, err) | ||
| ErrorLogger.Printf("Failed to copy file %s: %v", sanitizeLogInput(val.Filename), err) | ||
| } | ||
|
|
||
| err = f.Close() | ||
| if err != nil { | ||
| ErrorLogger.Printf("Failed to close file %s: %v", val.Filename, err) | ||
| ErrorLogger.Printf("Failed to close file %s: %v", sanitizeLogInput(val.Filename), err) | ||
| } | ||
| } | ||
| } |
| err = f.Close() | ||
| if err != nil { | ||
| ErrorLogger.Printf("Failed to close file %s: %v", sanitizeLogInput(val.Filename), err) | ||
| ErrorLogger.Printf("Failed to close file %s: %v", val.Filename, err) |
Check failure
Code scanning / CodeQL
Log entries created from user input High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months ago
To fix this problem, we should sanitize any user-controlled inputs before using them in log entries. Specifically, any place where val.Filename (coming from a multipart upload) is interpolated into a log should wrap it with the project-provided sanitizeLogInput function, which removes newline and carriage return characters. No changes are needed elsewhere in the code. The fix is targeted at the ErrorLogger.Printf("Failed to close file %s: %v", val.Filename, err) statement at line 641.
No new imports or external libraries are required, as sanitizeLogInput is already available within the file.
| @@ -617,13 +617,13 @@ | ||
| for _, val := range request.MultipartForm.File[fileKey] { | ||
| f, err := val.Open() | ||
| if err != nil { | ||
| ErrorLogger.Printf("Failed to open file %s: %v", val.Filename, err) | ||
| ErrorLogger.Printf("Failed to open file %s: %v", sanitizeLogInput(val.Filename), err) | ||
| continue | ||
| } | ||
|
|
||
| field, err := writer.CreatePart(val.Header) | ||
| if err != nil { | ||
| ErrorLogger.Printf("Failed to create part for file %s: %v", val.Filename, err) | ||
| ErrorLogger.Printf("Failed to create part for file %s: %v", sanitizeLogInput(val.Filename), err) | ||
| err = f.Close() | ||
| if err != nil { | ||
| ErrorLogger.Printf("Failed to close file: %v", err) | ||
| @@ -633,12 +628,12 @@ | ||
|
|
||
| _, err = io.Copy(field, f) | ||
| if err != nil { | ||
| ErrorLogger.Printf("Failed to copy file %s: %v", val.Filename, err) | ||
| ErrorLogger.Printf("Failed to copy file %s: %v", sanitizeLogInput(val.Filename), err) | ||
| } | ||
|
|
||
| err = f.Close() | ||
| if err != nil { | ||
| ErrorLogger.Printf("Failed to close file %s: %v", val.Filename, err) | ||
| ErrorLogger.Printf("Failed to close file %s: %v", sanitizeLogInput(val.Filename), err) | ||
| } | ||
| } | ||
| } |
No description provided.