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

Incorrect line numbers and missing line content when scanning Helm charts #6377

Closed
HenrikAlexisNilsson opened this issue May 12, 2023 · 0 comments · Fixed by #6611
Closed
Labels
bug Something isn't working community Community contribution

Comments

@HenrikAlexisNilsson
Copy link

Expected Behavior

When scanning a Helm chart using the Kics tool, each vulnerability found should be accompanied by the correct line number and line content.

Actual Behavior

However, in the case where a file within the Helm chart contains multiple vulnerabilities, only one of the vulnerabilities displays the correct line number and line content. All other vulnerabilities in the same file appear with line number 1 and no line content.

image

The log is also showing that it failed to detect the vulnerable line.

image

Cause of the Issue:
This issue appears to be a result of a regression introduced in the following commit on this line. The problem lies in the mutation of the lines variable on this line. Prior to this change, the mutation only affected the variable within the function, which was acceptable. However, with the recent commit, the lines variable is now a pointer referencing all the lines in the file. Consequently, any changes to it will have unintended consequences when the same function is called multiple times.

Explanation:
As a result, the initial call to retrieve the line number and line content successfully removes the # KICS_HELM_ID_ auxiliary lines from the lines variable (so they don't show up in the CLI output). However, subsequent calls to the function no longer have the # KICS_HELM_ID_ in the lines variable, causing the search for the vulnerable line to fail. As a fallback, these subsequent vulnerabilities are displayed with line number 1 and no line content and a warning is logged as Failed to detect line, query response....

Potential solution:
One solution would be to change this line from:

lines := *file.LinesOriginalData

to

lines := make([]string, len(*file.LinesOriginalData))
copy(lines, *file.LinesOriginalData)

This solution ensures that mutations to the lines variable won't affect file.LinesOriginalData. However, it may consume more memory due to the copy operation. I'm not familiar with Golang nor this repository so maybe there is a better solution to this issue?

Here is the result with the solution above being implemented:

image

Steps to Reproduce the Problem

  1. Download the Helm chart from here test-src.zip
  2. Unzip the archive unzip test-src.zip
  3. docker run -t -v ./test-src:/path checkmarx/kics:v1.7.1 scan -p /path/ -v --preview-lines 20
  4. You should see a log output similar to this log.txt where line contents and line number should be missing for all vulnerabilities except one and there should be warnings logged that it failed to detect the vulnerable line.
@HenrikAlexisNilsson HenrikAlexisNilsson added bug Something isn't working community Community contribution labels May 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working community Community contribution
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant