ThreatModels: Add stdin kind

[RasmusWL]

As discussed internally, none of the current local subgroups precisely captures stdin, so although it's much like both commandargs and file, a separate kind seems better.

I've ported the obvious models from C# and Java.

[RasmusWL]

I don't think this warrants a change-note, but let me know if you think otherwise 👍

[github-actions]

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

csharp

Generated file changes for csharp

• Changes to framework-coverage-csharp.csv:

- package,sink,source,summary,sink:code-injection,sink:encryption-decryptor,sink:encryption-encryptor,sink:encryption-keyprop,sink:encryption-symmetrickey,sink:file-content-store,sink:html-injection,sink:js-injection,sink:log-injection,sink:sql-injection,source:commandargs,source:database,source:environment,source:file,source:file-write,source:local,source:remote,source:windows-registry,summary:taint,summary:value
+ package,sink,source,summary,sink:code-injection,sink:encryption-decryptor,sink:encryption-encryptor,sink:encryption-keyprop,sink:encryption-symmetrickey,sink:file-content-store,sink:html-injection,sink:js-injection,sink:log-injection,sink:sql-injection,source:commandargs,source:database,source:environment,source:file,source:file-write,source:remote,source:stdin,source:windows-registry,summary:taint,summary:value
- Amazon.Lambda.APIGatewayEvents,,6,,,,,,,,,,,,,,,,,,6,,,
+ Amazon.Lambda.APIGatewayEvents,,6,,,,,,,,,,,,,,,,,6,,,,
- System,54,47,10626,,6,5,5,,,4,1,,33,2,,6,15,17,3,4,,8721,1905
+ System,54,47,10626,,6,5,5,,,4,1,,33,2,,6,15,17,4,3,,8721,1905

[michaelnebel]

I don't think this warrants a change-note, but let me know if you think otherwise 👍

Thank you @RasmusWL !

Just to be on the safe side, I think we should add a change note. At least for Java we are "moving" System.in into a new threat model group (for C# it seems less important with a change note as we are just making the modelling more fine grained).
Some tests that uses specific threat model groups might need an update (the threat model configuration is located in the .ext.yml file next to the test file).

[egregius313]

Overall I think this is a good decision.

But I'm debating whether it makes more sense as a completely separate threat model kind versus a sub-kind of file (my understanding of how threatModelEnabled works is that it supports sub-kinds).

Either way I agree with @michaelnebel that this should have a change note.

[owen-mc]

I agree it would be good to have a change note so that users who read the change notes are aware of the new threat model kind. You can probably copy some of the test from this change note from a PR where I added a new kind (but in a slightly different context, where I was removing some sources from the default and letting users know how to turn them back on).

I find @egregius313 's suggestion of making it a sub-model of file interesting. But I don't know enough about appsec to say what makes the most sense.

[owen-mc]

Please also update the documentation listing different threat model kinds here.

[RasmusWL]

I find it odd that I had to change MaD numbering in 224fe12, and I'm worried it's an indicator of an underlying problem (why would the numbering change for other parts of the modeling, when I'm only changing the threat-model of some stuff??) -- but I don't have enough expertise to debug this further. I did evaluate sourceModel locally on a random C# DB and confirmed that the 3 stdin models did not change madId after the threat-model was changed to stdin.

Had to force push after #17208 was merged, so disregard this.

[michaelnebel]

I find it odd that I had to change MaD numbering in 224fe12, and I'm worried it's an indicator of an underlying problem (why would the numbering change for other parts of the modeling, when I'm only changing the threat-model of some stuff??) -- but I don't have enough expertise to debug this further. I did evaluate sourceModel locally on a random C# DB and confirmed that the 3 stdin models did not change madId after the threat-model was changed to stdin.

I think the reason you are seeing a "shift" in the MaD IDs is because they are based directly on the QlBuiltins::ExtensionId, and the PR adds an extension row (the stdin) threat model group.
In any case you need to rebase the PR: I re-wrote all the C# tests yesterday to use the new post processing logic, which should increase test stability as the extension id (built-in) is no longer used directly in the expected test output.

[RasmusWL]

the PR adds an extension row (the stdin) threat model group.

Ah yes, that explains the difference 👍 I've rebased the PR 👍

[michaelnebel]

@RasmusWL : Excellent! Thank you!

[RasmusWL]

The C# Integration tests MacOS seems like a flaky failure that can be ignored, so should be ready to be merged now 👍

[michaelnebel]

Looks great!