-
Notifications
You must be signed in to change notification settings - Fork 6
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
NEST-536: Add necessary security exceptions #403
Conversation
Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs
Outdated
Show resolved
Hide resolved
// There is no need to security scan anything in Lombiq.Tests.UI.Shortcuts. | ||
configuration.ExcludeUrlWithRegex(@".*/Lombiq.Tests.UI.Shortcuts/.*"); | ||
|
||
configuration.MarkScanRuleAsFalsePositiveForUrlWithRegex( | ||
".*/(Login|ChangePassword)[?][rR]eturnUrl=.*", // #spell-check-ignore-line | ||
6, | ||
"Path Traversal", | ||
"Setting the ReturnUrl query parameter to a itself yields a false positive"); | ||
|
||
configuration.MarkScanRuleAsFalsePositiveForUrlWithRegex( | ||
".*/(Login|ChangePassword)[?][rR]eturnUrl=.*", // #spell-check-ignore-line | ||
40018, | ||
"SQL Injection", | ||
"Setting the ReturnUrl query parameter to an SQL expression can't actually cause SQL Injection."); |
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.
These three should rather be in the plans, so they're applied to every scan.
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.
You didn't yet move configuration.ExcludeUrlWithRegex(@".*/Lombiq.Tests.UI.Shortcuts/.*");
BTW.
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 don't know how to do that for a filter with no rule ID. Also the configuration.ExcludeUrlWithRegex(@".*/Admin/.*");
right above it is the same kind, so I assumed if it was possible you would've already moved it into the plans.
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.
In the end, this is run, what should be possible in YML too: https://github.com/Lombiq/UI-Testing-Toolbox/blob/dev/Lombiq.Tests.UI/SecurityScanning/YamlDocumentExtensions.cs#L107
I don't remember the reason for the admin config, but that perhaps makes more sense here: keep in mind that it's easier to override the C# config from a delegate, than modify the YML document. Maybe we want to override that. I can't imagine this for the shortcuts rule, though.
…Extensions.cs Co-authored-by: Zoltán Lehóczky <[email protected]>
NEST-536