Repository navigation
Conversation
Signed-off-by: Fynn Thierling <fynnth@outlook.de>
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
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.
Hi @n1ckl0sk0rtge,
As mentioned i found a small "bug" in the output module while working on csharp:
The problem
When the same algorithm shows up at two places in the code, both detections end up as one CBOM
component and they share a name, so they share an identity.
Until now only their source locations were combined. Everything else about the second one was
thrown away.
That loses real information. A cipher built twice from the same string, once to encrypt and once to
decrypt, ended up listed as doing only one of the two. The CBOM didn't just say less than it could,
it said something untrue.
The fix
I kept merging the locations as before, and also combine the properties that are lists ( the crypto
functions and the certification level). If one occurrence encrypts and another decrypts, the
component now says both.
Properties that describe a single call site, like mode or padding or key length, are deliberately
not carried over. One call site's configuration shouldn't be claimed for the others.
Why it's safe (Or why i believe so, please have a look here)
The location merge is the same code as before, just moved into its own method and given a null
check it was missing. Everything else only ever adds: for any input, the result is what you got
before plus whatever the second detection knew. Nothing can come out smaller than it did.
Since there were no unit tests for the CBOM content at all, I added a few for this change, so I can't claim the behaviour is proven for every language, but I could not find examples of it being wrong with the new merge method.
Best regards,
Fynn