-
Notifications
You must be signed in to change notification settings - Fork 118
chore: improve error logging #98
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
userAgent := request.Header.Get("User-Agent") | ||
|
||
// Log incoming request | ||
InfoLogger.Printf("Received %s request from %s for %s (User-Agent: %s)", method, clientIP, requestURL, userAgent) |
Check failure
Code scanning / CodeQL
Log entries created from user input High
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
To fix the problem, we need to sanitize the requestURL
before logging it. This can be done by removing any newline characters from the requestURL
to prevent log injection attacks. We will use the strings.ReplaceAll
function to replace newline characters with an empty string.
-
Copy modified lines R329-R330
@@ -328,3 +328,4 @@ | ||
method := request.Method | ||
requestURL := request.URL.String() | ||
requestURL := strings.ReplaceAll(request.URL.String(), "\n", "") | ||
requestURL = strings.ReplaceAll(requestURL, "\r", "") | ||
userAgent := request.Header.Get("User-Agent") |
userAgent := request.Header.Get("User-Agent") | ||
|
||
// Log incoming request | ||
InfoLogger.Printf("Received %s request from %s for %s (User-Agent: %s)", method, clientIP, requestURL, userAgent) |
Check failure
Code scanning / CodeQL
Log entries created from user input High
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
To fix the problem, we need to sanitize the userAgent
value before logging it. This can be done by removing any newline characters from the userAgent
string to prevent log forgery. We will use the strings.ReplaceAll
function to replace newline characters with an empty string. This ensures that the log entry remains a single line and cannot be manipulated by a malicious user.
-
Copy modified lines R331-R333
@@ -330,2 +330,5 @@ | ||
userAgent := request.Header.Get("User-Agent") | ||
// Sanitize userAgent to prevent log forgery | ||
userAgent = strings.ReplaceAll(userAgent, "\n", "") | ||
userAgent = strings.ReplaceAll(userAgent, "\r", "") | ||
|
if err != nil { | ||
ErrorLogger.Printf("Failed to write error response: %v", err) | ||
} | ||
ErrorLogger.Printf("Denied access to %s from disallowed origin: %s", clientIP, origin) |
Check failure
Code scanning / CodeQL
Log entries created from user input High
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
To fix the problem, we need to sanitize the origin
value before logging it. This can be done by removing any newline characters from the origin
string to prevent log injection. We will use the strings.ReplaceAll
function to replace newline characters with an empty string. This ensures that the log entry cannot be manipulated by injecting new lines.
We will make the following changes in the file libproxy/proxy.go
:
- Sanitize the
origin
value by removing newline characters before logging it.
-
Copy modified lines R361-R363
@@ -360,3 +360,5 @@ | ||
} | ||
ErrorLogger.Printf("Denied access to %s from disallowed origin: %s", clientIP, origin) | ||
sanitizedOrigin := strings.ReplaceAll(origin, "\n", "") | ||
sanitizedOrigin = strings.ReplaceAll(sanitizedOrigin, "\r", "") | ||
ErrorLogger.Printf("Denied access to %s from disallowed origin: %s", clientIP, sanitizedOrigin) | ||
return |
// If it is not an allowed origin, redirect back to hoppscotch.io. | ||
response.Header().Add("Location", "https://hoppscotch.io/") | ||
response.WriteHeader(301) | ||
InfoLogger.Printf("Redirected request from %s with disallowed origin: %s", clientIP, origin) |
Check failure
Code scanning / CodeQL
Log entries created from user input High
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
To fix the problem, we need to sanitize the origin
value before logging it. This can be done by removing any newline characters from the origin
string to prevent log forgery. We will use the strings.ReplaceAll
function to replace newline characters with an empty string. This ensures that the log entry cannot be manipulated by injecting new lines.
-
Copy modified lines R351-R352 -
Copy modified line R363 -
Copy modified line R369 -
Copy modified lines R373-R374
@@ -350,2 +350,4 @@ | ||
origin := request.Header.Get("Origin") | ||
sanitizedOrigin := strings.ReplaceAll(origin, "\n", "") | ||
sanitizedOrigin = strings.ReplaceAll(sanitizedOrigin, "\r", "") | ||
if origin == "" || !isAllowedOrigin(origin) { | ||
@@ -360,3 +362,3 @@ | ||
} | ||
ErrorLogger.Printf("Denied access to %s from disallowed origin: %s", clientIP, origin) | ||
ErrorLogger.Printf("Denied access to %s from disallowed origin: %s", clientIP, sanitizedOrigin) | ||
return | ||
@@ -366,3 +368,3 @@ | ||
response.WriteHeader(301) | ||
InfoLogger.Printf("Redirected request from %s with disallowed origin: %s", clientIP, origin) | ||
InfoLogger.Printf("Redirected request from %s with disallowed origin: %s", clientIP, sanitizedOrigin) | ||
return | ||
@@ -370,4 +372,4 @@ | ||
// Otherwise set the appropriate CORS policy and continue. | ||
response.Header().Add("Access-Control-Allow-Origin", origin) | ||
DebugLogger.Printf("Allowed request from origin: %s", origin) | ||
response.Header().Add("Access-Control-Allow-Origin", sanitizedOrigin) | ||
DebugLogger.Printf("Allowed request from origin: %s", sanitizedOrigin) | ||
} |
return | ||
} else { | ||
// Otherwise set the appropriate CORS policy and continue. | ||
response.Header().Add("Access-Control-Allow-Origin", request.Header.Get("Origin")) | ||
response.Header().Add("Access-Control-Allow-Origin", origin) | ||
DebugLogger.Printf("Allowed request from origin: %s", origin) |
Check failure
Code scanning / CodeQL
Log entries created from user input High
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
To fix the problem, we need to sanitize the origin
value before logging it. This can be done by removing any newline characters and other potentially harmful characters from the origin
string. We can use the strings.ReplaceAll
function to replace newline characters with an empty string. This ensures that the log entries are safe and cannot be manipulated by user input.
-
Copy modified lines R351-R353 -
Copy modified line R370 -
Copy modified line R375
@@ -350,2 +350,5 @@ | ||
origin := request.Header.Get("Origin") | ||
// Sanitize the origin value to prevent log forgery | ||
sanitizedOrigin := strings.ReplaceAll(origin, "\n", "") | ||
sanitizedOrigin = strings.ReplaceAll(sanitizedOrigin, "\r", "") | ||
if origin == "" || !isAllowedOrigin(origin) { | ||
@@ -366,3 +369,3 @@ | ||
response.WriteHeader(301) | ||
InfoLogger.Printf("Redirected request from %s with disallowed origin: %s", clientIP, origin) | ||
InfoLogger.Printf("Redirected request from %s with disallowed origin: %s", clientIP, sanitizedOrigin) | ||
return | ||
@@ -371,3 +374,3 @@ | ||
response.Header().Add("Access-Control-Allow-Origin", origin) | ||
DebugLogger.Printf("Allowed request from origin: %s", origin) | ||
DebugLogger.Printf("Allowed request from origin: %s", sanitizedOrigin) | ||
} |
|
||
DebugLogger.Printf("Sending proxied request to %s", proxyRequest.URL.String()) |
Check failure
Code scanning / CodeQL
Log entries created from user input High
user-provided value
This log entry depends on a
user-provided value
This log entry depends on a
user-provided value
This log entry depends on a
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
To fix the problem, we need to sanitize the user input before logging it. Specifically, we should ensure that any user-provided URL is properly escaped to prevent log injection attacks. We can use the strings.ReplaceAll
function to remove any newline characters from the URL before logging it. This will prevent a malicious user from injecting new log entries or manipulating the log output.
- Identify the lines where user input is logged.
- Sanitize the user input by removing newline characters.
- Replace the original log statements with the sanitized input.
-
Copy modified lines R600-R602
@@ -599,3 +599,5 @@ | ||
|
||
DebugLogger.Printf("Sending proxied request to %s", proxyRequest.URL.String()) | ||
sanitizedURL := strings.ReplaceAll(proxyRequest.URL.String(), "\n", "") | ||
sanitizedURL = strings.ReplaceAll(sanitizedURL, "\r", "") | ||
DebugLogger.Printf("Sending proxied request to %s", sanitizedURL) | ||
proxyStartTime := time.Now() |
log.Print("Failed to write response body: ", err.Error()) | ||
_, _ = fmt.Fprintln(response, ErrorBodyProxyRequestFailed) | ||
atomic.AddUint64(&totalErrors, 1) | ||
ErrorLogger.Printf("Failed to execute proxied request to %s: %v", proxyRequest.URL.String(), err) |
Check failure
Code scanning / CodeQL
Log entries created from user input High
user-provided value
This log entry depends on a
user-provided value
This log entry depends on a
user-provided value
This log entry depends on a
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
To fix the problem, we need to sanitize the user input before logging it. Specifically, we should remove any newline characters from the URL to prevent log forgery. We can use the strings.ReplaceAll
function to achieve this. This change should be made in the libproxy/proxy.go
file where the logging occurs.
-
Copy modified lines R607-R609
@@ -606,3 +606,5 @@ | ||
atomic.AddUint64(&totalErrors, 1) | ||
ErrorLogger.Printf("Failed to execute proxied request to %s: %v", proxyRequest.URL.String(), err) | ||
sanitizedURL := strings.ReplaceAll(proxyRequest.URL.String(), "\n", "") | ||
sanitizedURL = strings.ReplaceAll(sanitizedURL, "\r", "") | ||
ErrorLogger.Printf("Failed to execute proxied request to %s: %v", sanitizedURL, err) | ||
_, writeErr := fmt.Fprintln(response, "{\"success\": false, \"data\":{\"message\":\"(Proxy Error) Request failed: "+err.Error()+"\"}}") |
}(proxyResponse.Body) | ||
|
||
InfoLogger.Printf("Received response from %s with status %d in %v", | ||
proxyRequest.URL.String(), |
Check failure
Code scanning / CodeQL
Log entries created from user input High
user-provided value
This log entry depends on a
user-provided value
This log entry depends on a
user-provided value
This log entry depends on a
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
To fix the problem, we need to sanitize the user input before logging it. Specifically, we should ensure that any user-provided URL is properly escaped to prevent log forgery or injection. We can use the strings.ReplaceAll
function to remove any newline characters from the URL before logging it.
- Identify the lines where user input is logged.
- Sanitize the user input by removing newline characters.
- Replace the original log statements with the sanitized input.
-
Copy modified lines R623-R624 -
Copy modified line R626
@@ -622,4 +622,6 @@ | ||
|
||
sanitizedURL := strings.ReplaceAll(proxyRequest.URL.String(), "\n", "") | ||
sanitizedURL = strings.ReplaceAll(sanitizedURL, "\r", "") | ||
InfoLogger.Printf("Received response from %s with status %d in %v", | ||
proxyRequest.URL.String(), | ||
sanitizedURL, | ||
proxyResponse.StatusCode, |
|
||
// Log completion | ||
InfoLogger.Printf("Completed %s request from %s to %s in %v", | ||
requestData.Method, |
Check failure
Code scanning / CodeQL
Log entries created from user input High
user-provided value
This log entry depends on a
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
To fix the problem, we need to sanitize the user input before logging it. Specifically, we should remove any newline characters from the requestData.Method
, clientIP
, and requestData.Url
to prevent log injection attacks. This can be done using the strings.ReplaceAll
function to replace newline characters with an empty string.
-
Copy modified lines R682-R689 -
Copy modified lines R691-R693
@@ -681,6 +681,14 @@ | ||
// Log completion | ||
// Sanitize user input before logging | ||
sanitizedMethod := strings.ReplaceAll(requestData.Method, "\n", "") | ||
sanitizedMethod = strings.ReplaceAll(sanitizedMethod, "\r", "") | ||
sanitizedClientIP := strings.ReplaceAll(clientIP, "\n", "") | ||
sanitizedClientIP = strings.ReplaceAll(sanitizedClientIP, "\r", "") | ||
sanitizedUrl := strings.ReplaceAll(requestData.Url, "\n", "") | ||
sanitizedUrl = strings.ReplaceAll(sanitizedUrl, "\r", "") | ||
|
||
InfoLogger.Printf("Completed %s request from %s to %s in %v", | ||
requestData.Method, | ||
clientIP, | ||
requestData.Url, | ||
sanitizedMethod, | ||
sanitizedClientIP, | ||
sanitizedUrl, | ||
time.Since(startTime)) |
InfoLogger.Printf("Completed %s request from %s to %s in %v", | ||
requestData.Method, | ||
clientIP, | ||
requestData.Url, |
Check failure
Code scanning / CodeQL
Log entries created from user input High
user-provided value
This log entry depends on a
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
To fix the problem, we need to sanitize the requestData.Url
before logging it. This can be done by removing any newline characters from the URL to prevent log injection attacks. We will use the strings.ReplaceAll
function to replace newline characters with an empty string.
-
Copy modified lines R682-R684 -
Copy modified line R688
@@ -681,2 +681,5 @@ | ||
// Log completion | ||
// Sanitize requestData.Url to prevent log injection | ||
sanitizedUrl := strings.ReplaceAll(requestData.Url, "\n", "") | ||
sanitizedUrl = strings.ReplaceAll(sanitizedUrl, "\r", "") | ||
InfoLogger.Printf("Completed %s request from %s to %s in %v", | ||
@@ -684,3 +687,3 @@ | ||
clientIP, | ||
requestData.Url, | ||
sanitizedUrl, | ||
time.Since(startTime)) |
Description
This PR is responsible for better logging of proxyscotch.
Checks