Fix wrong line number in private key detector (#4485) #4486
Conversation
Private key detector findings report wrong line number when private key literal doesn't end with new line character. After private is matched using regexp is goes through Normalize function and normalized result is used in result.Raw and then used in engine.FragmentLineOffset which looks for line of code. Normalization step is crucial as ssh.ParseRawPrivateKey is quite strict about format of accepted key and this step can sieve false posivites as it can verify it private key is legit or just matches permisive regexp. Normalize always adds newline char at end of string (as needed for validation) but such string, with new line at the end is then used for looking for LOC. If source chunk didn't have new line char right after private key engine will report default LOC. This fix changes Result.Raw for private key detector to use raw match from regexp and not normalized string. This way engine can calculate correct LOC for such finding.
| s1 := detectors.Result{ | ||
| DetectorType: detectorspb.DetectorType_PrivateKey, | ||
| Raw: []byte(token), | ||
| Raw: []byte(match), |
There was a problem hiding this comment.
We don't recommend updating the Raw field value, as it's used as an identifier and changing it can interfere with existing findings. Your solution is correct, but instead, you can use:
s1.SetPrimarySecretValue(match)This will instruct the engine to use match to determine the line number.
…cret (trufflesecurity#4485) Revert previous changes that changed result.Raw in primary key detector as it can interfere with existing finding. Use SetPrimarySecretValue(match) instead
|
@trufflesecurity/backend This PR involves a detector change, so I’m not sure why the backend team is the owner. Could someone from the Backend team please review and merge it? |
@trufflesecurity/backend owns critical detectors |
rosecodym
left a comment
rosecodym
left a comment
There was a problem hiding this comment.
Thanks for this! I'm mostly deferring to @kashifkhan0771 :)
4 participants
Description:
Private key detector findings report wrong line number when private key literal doesn't end with new line character.
After private is matched using regexp is goes through Normalize function and normalized result is used in result.Raw and then used in engine.FragmentLineOffset which looks for line of code.
Normalization step is crucial as ssh.ParseRawPrivateKey is quite strict about format of accepted key and this step can sieve false posivites as it can verify it private key is legit or just matches permisive regexp.
Normalize always adds newline char at end of string (as needed for validation) but such string, with new line at the end is then used for looking for LOC. If source chunk didn't have new line char right after private key engine will report default LOC.
This fix changes Result.Raw for private key detector to use raw match from regexp and not normalized string. This way engine can calculate correct LOC for such finding.
Related issue #4485
Checklist:
make test-community)?make lintthis requires golangci-lint)? - it doesn't even pass for main branch locally