fix(rules): improve precision of 4 high-FP dotnet opengrep rules#63
Open
David Larsen (dc-larsen) wants to merge 1 commit intomainfrom
Open
fix(rules): improve precision of 4 high-FP dotnet opengrep rules#63David Larsen (dc-larsen) wants to merge 1 commit intomainfrom
David Larsen (dc-larsen) wants to merge 1 commit intomainfrom
Conversation
David Larsen (dc-larsen)
changed the title
Addresses customer SAST evaluation feedback where 4 rules produced 150/170 false positives (88% of all FPs), inflating the reported FP rate to 91%. Rules fixed: - dotnet-xss-response-write: Convert to taint mode. Previously matched any .Write() call including Serilog ITextFormatter log sinks. Now requires data flow from user input sources to Response.Write sinks. - dotnet-hardcoded-credentials: Add value inspection and credential API patterns. Previously matched on variable names alone, flagging config key paths like "UseCaptchaOnResetPassword". - dotnet-crypto-failures: Target actual weak algorithms (3DES, DES, RC2, RijndaelManaged) instead of Encoding.UTF8.GetBytes() which flagged the recommended SHA256.HashData(Encoding.UTF8.GetBytes(...)) pattern. - dotnet-path-traversal: Convert to taint mode. Previously matched all Path.Combine() calls including those using framework-provided paths like _env.WebRootPath. Validated with opengrep v1.19.0 against NIST Juliet C# test suite: xss-response-write: Prec 41.6% -> 100%, Recall 47.8% -> 24.3% hardcoded-credentials: Prec 0.0% -> 100%, Recall 0.0% -> 3.6% crypto-failures: Prec 36.7% -> 100%, Recall 51.4% -> 50.0% path-traversal: Prec 0.0% -> 100%, Recall 0.0% -> 45.2%
David Larsen (dc-larsen)
force-pushed
the
fix/dotnet-sast-rule-precision
branch
from
4958b6f to
cdb7224
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes 4 dotnet opengrep rules that produced 150 of 170 total false positives (88%) in a customer SAST evaluation, inflating the reported FP rate to 91%.
.Write()including SerilogITextFormatterlog sinks (74 FPs). Now tracks data flow from user input toResponse.Write.UseCaptchaOnResetPassword(31 FPs).Encoding.UTF8.GetBytes()which triggers on the recommendedSHA256.HashData()pattern (30 FPs).Path.Combine()calls including framework paths like_env.WebRootPath(15 FPs).Benchmark data (NIST Juliet C# Test Suite)
All 4 rules achieve 100% precision (zero false positives) post-fix. Recall trade-offs are acceptable: taint-mode rules only fire when user input actually reaches the sink, which is the correct behavior for security analysis.
Customer impact
Eliminates all 150 FPs from these 4 rules. Remaining findings (36 total, 20 FP) produce a ~56% FP rate, consistent with pattern-matching SAST tools. Further tuning via community rules and per-language scoping can reduce this further.
Testing
opengrep --validatepasses on full dotnet.yml (40 rules, 0 errors)pytestpasses (139 tests, 0 failures)