Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(secret): trim excessively long lines #7192

Merged
merged 7 commits into from
Jul 23, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions pkg/fanal/secret/scanner.go
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,10 @@ func toFinding(rule Rule, loc Location, content []byte) types.SecretFinding {
}
}

const secretHighlightRadius = 2 // number of lines above + below each secret to include in code output
const (
secretHighlightRadius = 2 // number of lines above + below each secret to include in code output
maxLineLength = 2000 //
)

func findLocation(start, end int, content []byte) (int, int, types.Code, string) {
startLineNum := bytes.Count(content[:start], lineSep)
Expand Down Expand Up @@ -511,9 +514,16 @@ func findLocation(start, end int, content []byte) (int, int, types.Code, string)
rawLines := lines[codeStart:codeEnd]
var foundFirst bool
for i, rawLine := range rawLines {
strRawLine := string(rawLine)
realLine := codeStart + i
inCause := realLine >= startLineNum && realLine <= endLineNum

var strRawLine string
if len(rawLine) > maxLineLength {
strRawLine = lo.Ternary(inCause, matchLine, string(rawLine[:maxLineLength]))
} else {
strRawLine = string(rawLine)
}
Comment on lines +521 to +525
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like 100 characters is enough.

  1. For lines before and after secret - we don't need such a big line.
  2. For secret line - if length of secret with secret > 100 - we use 30 + secret + 20 characters:
    if lineEnd-lineStart > 100 {
    lineStart = lo.Ternary(start-30 < 0, 0, start-30)
    lineEnd = lo.Ternary(end+20 > len(content), len(content), end+20)
    }

Now 2000 is a length for backward compatibility with RSA keys.

IIUC you want to include full RSA key to result (-----BEGIN RSA PRIVATE KEY----- and -----END RSA PRIVATE KEY-----), but I think this is not necessary.
But when we use 2000, we can see case when secret length is 10, but the user sees 1900 unneeded characters.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DmitriyLewen I generally agree with you. 2k is too long line.


code.Lines = append(code.Lines, types.Line{
Number: codeStart + i + 1,
Content: strRawLine,
Expand Down
30 changes: 30 additions & 0 deletions pkg/fanal/secret/scanner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -667,6 +667,27 @@ func TestSecretScanner(t *testing.T) {
},
},
}
wantFindingTokenInsideJs := types.SecretFinding{
RuleID: "stripe-publishable-token",
Category: "Stripe",
Title: "Stripe Publishable Key",
Severity: "LOW",
StartLine: 1,
EndLine: 1,
Match: "){case a.ez.PRODUCTION:return\"********************************\";case a.ez.TEST:cas",
Code: types.Code{
Lines: []types.Line{
{
Number: 1,
Content: "){case a.ez.PRODUCTION:return\"********************************\";case a.ez.TEST:cas",
Highlighted: "){case a.ez.PRODUCTION:return\"********************************\";case a.ez.TEST:cas",
IsCause: true,
FirstCause: true,
LastCause: true,
},
},
},
}

tests := []struct {
name string
Expand Down Expand Up @@ -982,6 +1003,15 @@ func TestSecretScanner(t *testing.T) {
Findings: []types.SecretFinding{wantMultiLine},
},
},
{
name: "long obfuscated js code with secrets",
configPath: filepath.Join("testdata", "skip-test.yaml"),
inputFilePath: filepath.Join("testdata", "obfuscated.js"),
want: types.Secret{
FilePath: filepath.Join("testdata", "obfuscated.js"),
Findings: []types.SecretFinding{wantFindingTokenInsideJs},
},
},
}

for _, tt := range tests {
Expand Down
1 change: 1 addition & 0 deletions pkg/fanal/secret/testdata/obfuscated.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.