feat: C# support (System.Security.Cryptography) - initial draft#376
feat: C# support (System.Security.Cryptography) - initial draft#376
Conversation
Adds a first-pass C# language module (csharp/) using an ANTLR-based custom SonarQube sensor, following the pattern established by the Go sensor. Includes a custom tree representation, detection engine, and detection rules for the most common System.Security.Cryptography classes: AES, DES, TripleDES, RC2, RSA, DSA, ECDsa, ECDiffieHellman, HMAC variants, SHA hashing, and Rfc2898DeriveBytes. All detection rules have unit tests and have been validated end-to-end against a local SonarQube instance (23 findings produced). Signed-off-by: Fynn Thierling <Fynnth@outlook.de>
7a469e6 to
244d023
Compare
|
One thing I noticed while looking at the implementation more closely: property assignments like aes.Mode = CipherMode.CBC are currently not detected at all -> the tree converter only picks up method calls and constructors, so anything set via a property after object creation is invisible to the detection engine. This means mode, padding, and similar parameters are missed unless they're passed directly as constructor arguments. I'm not sure how to fix this properly. From what I can tell it would require either tracking the variable across statements (Symbol table) or using a different approach with actual semantic analysis. Any input on that? |
|
Hi @fynnth - Thanks a lot for the code drop. Amazing work! Do you have a C# test repo with a set of known findings a scsn should produce? I could give it a try it on a Mac. |
|
Hi @san-zrl, Also, since the parser is not using full c# grammar, many things in real world repos won't work. |
|
Hi @fynnth - I pulled your PR, built it and ran it in a docker container SonarQube instance on my system. I scanned the files in |
|
Thank you, that looks perfect! The coming week i will try to extend the parser to be more complete and maybe introduce a symbol table to also detect properties or similar things. There could still be some issues after that with properties outside of functions, but i think this will be a big step if i could achieve that :) Also i will try to integrate any feedback Nicklas still has! |
|
@fynnth some initial feedback. In general looks very good!!! Thanks for coding this up Revert
|
Yes, mostly. I think, just the
ANTLR is the right call here :) there's no sonar-csharp rule API to hook into (unlike Java/Python), so a custom parser is the only option.
No, look good! :)
Some thoughts:
In general, this repo might help https://github.com/FRI-DAY/sonar-gosu-plugin/tree/main as a reference. Its a sonar plugin based on ANTLR. |
Signed-off-by: Fynn Thierling <Fynnth@outlook.de>
|
Hey @n1ckl0sk0rtge , Thank you for the quick response! Addressed feedback:
Does everything fit what you had in mind? Question on Phase 2 (property assignment tracking): For patterns like aes.Mode = CipherMode.CBC / aes.KeySize = 256, i was thinking of extracting these as a new CSharpPropertyAssignmentTree node type in the converter, and then extending the detection engine to scan the block for property assignments on a tracked variable after an initial detection fires. Would this be the approach you'd recommend, or would you prefer to model property setters as synthetic method invocations (e.g. set_Mode(CipherMode.CBC)) so that existing withDependingDetectionRules machinery can be reused? Happy to go either direction or something completely different, because i really do not know what would be best. |
|
Hey @fynnth, thanks for adjusting this! :) Regarding the property assignment tracking, I'd think modeling property setters as method invocations sounds good, but I'm not 100% sure either :) C# properties are syntactic sugar for get_/set_ methods, so the compiler compiles into something like As i would see it, in The only thing to watch out for is that tree visitor visits these synthetic nodes the same way it visits real method invocations, so the depending rule chain picks them up correctly. |
…cations Model C# property assignments (e.g. ) as synthetic method invocations in the tree converter, following the CLR convention where properties compile to / methods. This enables the existing chain to detect property-based crypto configuration without any changes to the detection engine itself. Signed-off-by: Fynn Thierling <Fynnth@outlook.de>
Signed-off-by: Fynn Thierling <Fynnth@outlook.de>
|
Hi @n1ckl0sk0rtge, I have added some initial version for property tracking in the latest 2 commits (last one was only a quick assertion fix). Please check if you think it is enough for first detection rules or if i should focus more on detection capabilties on the engine side. Here a quick recap of what i think the current version is capable of and what not: Detection Capabilities: What is detectable now through the right detection rules:
What still is problematic from what i assume: Parser Issues:
Engine-Gaps:
Grammar-Gaps(Parser fails-> file will be skipped, since th grammar i am using is only complete for c# 6):
And I might miss many things here (i do not code in c#) so this is not a complete list. if you know of something, let me know! So i want to start with detection rules, but if you say we should complete the skeleton completely first and find some solution to the problems i understadn that and i will try harder to solve them in some way. (i dont know if that is easily done though for the issues with the antlr grammar) |
C# Support — Initial Draft (feat/csharp-support)
Hey TSC-Members, just putting this up as a draft so you can take a look at what I've been working on. This is not meant to be merged anytime soon and it's more of a "here's where I'm at, please tell me if I'm going in the right direction" kind of thing.
What's in here
I've added a first pass at C# /
System.Security.Cryptographysupport. The approach I went with is basically the same as what was discussed a year ago for parsing: a custom SonarQube sensor backed by an ANTLR grammar to parse C# source files (It is not yet a complete c# parser). The sensor hooks into the existing engine and detection rule infrastructure, so detection rules and the mapper layer stay consistent with the other languages.I was heaviy inspired by the recently added go support and had claude basically replicate all engine files and the sensor setup fitted for c#. And this seemed to work very good, but correct me on any mistake i made there.
There are a handful of detection rules for the most common
System.Security.Cryptographyclasses (AES, DES, RSA, HMAC variants, hashing, etc.) so that it's enough to validate that the whole pipeline works end to end.I tested this locally against SonarQube and the rules do produce findings, so at least the basic flow is working to some extend.
Things I'd specifically like @n1ckl0sk0rtge to look at
As I said, I worked on this partly together with Claude (first timew using it as well) and I'm honestly not 100% sure whether some of my/its structural choices align with how things are supposed to be done here. Specifically:
csharp/module, the sensor, etc.) fit the pattern you had in mind for new language support?No need to go deep on the detection rules themselves for now, since those will need to be expanded a lot anyway. I'm mostly asking about the skeleton.
Please test on another system if you can
I've only run this on my own machine (WSL2). It would be really helpful if someone could pull this branch, build it, and drop the JAR into a SonarQube instance to see whether the findings actually show up. Since I for example had some issues on windows some time earlier with some crlf vs lf file endings for the .sh scripts in this project and compilation works out of the box only on linux not on windows for me.
Build and deploy steps are the same as for the rest of the plugin -> build with
mvn clean package, then copy the JAR fromsonar-cryptography-plugin/target/into your SonarQube plugins folder and restart.Again, this is very much a draft. I am very happy to rework anything that's off or needs some fine tuning.