-
Notifications
You must be signed in to change notification settings - Fork 3
Fix code scanning alert no. 3: Log entries created from user input #9
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
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
| _logger.LogDebug("received request for {msisdn} {session_id} {gs_request}", request.Mobile, request.SessionId, | ||
| JsonConvert.SerializeObject(request)); | ||
| var sanitizedRequest = JsonConvert.SerializeObject(request).Replace(Environment.NewLine, "").Replace("\n", "").Replace("\r", ""); | ||
| _logger.LogDebug("received request for {msisdn} {session_id} {gs_request}", request.Mobile, request.SessionId, sanitizedRequest); |
Check failure
Code scanning / CodeQL
Log entries created from user input
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI over 1 year ago
To fix the problem, we need to sanitize the request.Mobile value before logging it. This can be done by removing any new line characters from the request.Mobile string. We will use the String.Replace method to ensure that no line endings are present in the user input. This will prevent any potential log forging or injection attacks.
-
Copy modified lines R45-R46
| @@ -44,3 +44,4 @@ | ||
| var sanitizedRequest = JsonConvert.SerializeObject(request).Replace(Environment.NewLine, "").Replace("\n", "").Replace("\r", ""); | ||
| _logger.LogDebug("received request for {msisdn} {session_id} {gs_request}", request.Mobile, request.SessionId, sanitizedRequest); | ||
| var sanitizedMobile = request.Mobile.Replace(Environment.NewLine, "").Replace("\n", "").Replace("\r", ""); | ||
| _logger.LogDebug("received request for {msisdn} {session_id} {gs_request}", sanitizedMobile, request.SessionId, sanitizedRequest); | ||
|
|
| _logger.LogDebug("received request for {msisdn} {session_id} {gs_request}", request.Mobile, request.SessionId, | ||
| JsonConvert.SerializeObject(request)); | ||
| var sanitizedRequest = JsonConvert.SerializeObject(request).Replace(Environment.NewLine, "").Replace("\n", "").Replace("\r", ""); | ||
| _logger.LogDebug("received request for {msisdn} {session_id} {gs_request}", request.Mobile, request.SessionId, sanitizedRequest); |
Check failure
Code scanning / CodeQL
Log entries created from user input
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI over 1 year ago
To fix the problem, we need to sanitize the SessionId before logging it. This can be done by removing any newline characters from the SessionId to prevent log forging. We will use the Replace method to remove newline characters from the SessionId before logging it.
-
Copy modified lines R45-R46
| @@ -44,3 +44,4 @@ | ||
| var sanitizedRequest = JsonConvert.SerializeObject(request).Replace(Environment.NewLine, "").Replace("\n", "").Replace("\r", ""); | ||
| _logger.LogDebug("received request for {msisdn} {session_id} {gs_request}", request.Mobile, request.SessionId, sanitizedRequest); | ||
| var sanitizedSessionId = request.SessionId.Replace(Environment.NewLine, "").Replace("\n", "").Replace("\r", ""); | ||
| _logger.LogDebug("received request for {msisdn} {session_id} {gs_request}", request.Mobile, sanitizedSessionId, sanitizedRequest); | ||
|
|
Fixes https://github.com/hubtel/programmable-services-sdk-dotnet/security/code-scanning/3
To fix the problem, we need to sanitize the user input before logging it. Since the log entries are plain text, we should remove any newline characters from the serialized JSON string to prevent log forging. This can be achieved using the
String.Replacemethod to replace newline characters with an empty string.Suggested fixes powered by Copilot Autofix. Review carefully before merging.