-
Notifications
You must be signed in to change notification settings - Fork 934
.NET: sanitize redirectUrl for logs #2356
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds security hardening to prevent log injection attacks by sanitizing the redirectUrl before logging. When redirecting requests to include a trailing slash, the URL is now sanitized to remove newline characters that could be used to inject malicious content into logs.
- Introduces a
GeneratedRegexpattern to match and remove newline characters (\r\n) - Changes the class to
partialto support the source-generated regex - Sanitizes the
redirectUrlbefore logging to prevent log injection vulnerabilities
ReubenBond
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not 100% sure on the right approach for sanitizing log lines. I think it's fine to just delete that log line, tbh. It's a false alarm anyway, because path == _basePath which is a constant and cannot contain newlines.
I also consider this a false alarm, but I think DevUI should have more logs, not less, to improve dev experience and debugging. That's why decided to leave it. Decided to do the simple regex for now - we can improve later once we find out best practice for sanitization |
Uh oh!
There was an error while loading. Please reload this page.