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

Extend the code snippet included in the issue and refactored how the code snippet is printed #497

Merged
merged 3 commits into from
Jul 7, 2020

Conversation

ccojocar
Copy link
Member

@ccojocar ccojocar commented Jun 25, 2020

fixes #268

This will capture 3 lines around the place where the issue occurs. The output looks something like:

[/Users/cosmin/go/src/github.com/securego/gosec/issue.go:180] - G307 (CWE-703): Deferring unsafe method "Close" on type "*os.File" (Confidence: HIGH, Severity: MEDIUM)
    179:        if file, err := os.Open(fobj.Name()); err == nil {
  > 180:                defer file.Close()
    181:                s := codeSnippetStartLine(node, fobj)



Summary:
   Files: 50
   Lines: 8069
   Nosec: 21
  Issues: 1

Also it marks properly a multiline finding:

[/Users/cosmin/go/src/github.com/securego/gosec/issue.go:170-171] - G102 (CWE-200): Binds to all network interfaces (Confidence: HIGH, Severity: MEDIUM)
    169: func multiline() {
  > 170:        _, _ = net.Listen("tcp",
  > 171:                "0.0.0.0:2000")
    172: }

In this way, it seems easier to figure out the place where the security issue is detected.

…code snippet is printed

Signed-off-by: Cosmin Cojocar <cosmin.cojocar@gmx.ch>
Signed-off-by: Cosmin Cojocar <cosmin.cojocar@gmx.ch>
@ccojocar ccojocar requested a review from gcmurphy June 25, 2020 15:05
@codecov-commenter
Copy link

codecov-commenter commented Jun 25, 2020

Codecov Report

Merging #497 into master will increase coverage by 0.94%.
The diff coverage is 92.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #497      +/-   ##
==========================================
+ Coverage   71.75%   72.70%   +0.94%     
==========================================
  Files           9        9              
  Lines         662      674      +12     
==========================================
+ Hits          475      490      +15     
+ Misses        164      163       -1     
+ Partials       23       21       -2     
Impacted Files Coverage Δ
issue.go 75.00% <92.00%> (+11.53%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3784ffe...622cb6f. Read the comment docs.

Signed-off-by: Cosmin Cojocar <cosmin.cojocar@gmx.ch>
Copy link
Member

@gcmurphy gcmurphy left a comment

Choose a reason for hiding this comment

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

Looks good.

(I didn't check if this caused any issues with other output formatters though)

@ccojocar
Copy link
Member Author

ccojocar commented Jul 6, 2020

on other format it generates something like this:

{
        "Golang errors": {},
        "Issues": [
                {
                        "severity": "MEDIUM",
                        "confidence": "HIGH",
                        "cwe": {
                                "ID": "703",
                                "URL": "https://cwe.mitre.org/data/definitions/703.html"
                        },
                        "rule_id": "G307",
                        "details": "Deferring unsafe method \"Close\" on type \"*os.File\"",
                        "file": "/Users/cosmin/go/src/github.com/securego/gosec/issue.go",
                        "code": "180: \tif file, err := os.Open(fobj.Name()); err == nil {\n181: \t\tdefer file.Close()\n182: \t\ts := codeSnippetStartLine(node, fobj)\n",
                        "line": "181",
                        "column": "3"
                }
        ],
        "Stats": {
                "files": 50,
                "lines": 8093,
                "nosec": 21,
                "found": 1
        }
}```

@ccojocar
Copy link
Member Author

ccojocar commented Jul 6, 2020

3 lines are taken from the code snippet and inclined with the line numbers.

@ccojocar ccojocar merged commit 6bcd89a into securego:master Jul 7, 2020
@ccojocar ccojocar deleted the extend-code-snippet-from-issue branch May 13, 2024 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add more lines in rule code section
3 participants