feat(csharp): add C# ChaCha20Poly1305 detection rules - #506

Hi Martin,

That looks good structure wise but we need to refine the detection rule a bit according to the comments above.
Take a look at the structure in the AES-Branch: https://github.com/fynnth/sonar-cryptography-csharp/tree/aes-related-crypto
The detection rules for csharp currently in https://github.com/cbomkit/sonar-cryptography are just dummies that need to be refined to not only detect only the constructor but also all operations that then depend on that instantiation. For chacha20poly1305 this should be quick like i outlined in the comments :)
If something is unclear, just ask.

Best regards,
fynn

Hi Martin,

that looks way better now, very good job! Also the unit test is comprehensive and covers properly.
@cbomkit/cbomkit-maintainers:
could you also have a look, if it fits for you? and should we merge this as a first proper detection rule for csharp or wait till the whole library will be covered?

Best regards,
Fynn

Hi @Kaldesyvon , hi @fynnth,

I just learned that C# has the same concept of named parameters like python. Named parameters in a function call can occur in arbitrary order. Some of them are optional which makes the number of variants a valid function call can have even higher. Writing detection rules for pycryptodome was thus very difficult because you need a lot of rules to cover the most common call variants. To this end, I suggested to add another call to the detection rule interface

.withNamedMethodParameter(String name, String, boolean optional);

withNamedMethodParameter can be used the same way as the original wtihNamedParameter call to match method parameters. It just uses the parameter names (if existing) as the first selection criterion (falling back to positional matching if there is no name). The named parameters can occur in any order. They can be combined with positional parameters (preceeding the named ones). Please have a look, this is implemented in PR #509.

For your reference, there is currently one detection rule in python/…/detection/pycrypto/hash/PycryptoCryptoHash.java for SHA512.new(data=None, truncate=None) as a sample rule for a function with named optional parameters. There is also extensive testing of edge cases which gives you an idea how this rules behaves, what matches and what doesn't. I asked @n1ckl0sk0rtge to review the PR. As i mentioned, this is a byproduct of #495 which is on hold until #509 is decided.

Hi @san-zrl,

Thats true, I have not thought of that. But for csharp the issue is even worse because the parameter name was not in the parser interface that I have implemented, so i have to rework that too before anything like that works. (info is just not there for parameter names)

My plan would be to finish detection rules like it is now and then afterwards start reworking the engine and implementing named parameters, then update the rules and also extend parameter detection in general (for example trying to detect certain key sizes that were instantiated in a variable before the method call)
Would that fit?

@fynnth - Sure. For the moment the new feature is python specific. We have to see where else it would make sense.

Hi @Kaldesyvon, @fynnth - What's the state of this? Are you still working on it?

Hi @san-zrl, I was on PTO and today I started resolving comments. I will notify you for another review

Hello @san-zrl @fynnth,

could you please do a review?

Thanks

Hi @Kaldesyvon @san-zrl ,

Thumbs up from my side, i just merged my detection rules and there is small conflict here which i will resolve and then we can merge it :)