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

[Bug BLOCKER] Ignoring vuln_id with a json file doesn't work. #16

Closed
Yivan opened this issue Feb 17, 2020 · 4 comments
Closed

[Bug BLOCKER] Ignoring vuln_id with a json file doesn't work. #16

Yivan opened this issue Feb 17, 2020 · 4 comments

Comments

@Yivan
Copy link

Yivan commented Feb 17, 2020

Hello,

Thanks for providing to us this nice security package.

I use last phar version, and set the config file like this:
setFalsePositives: "./tests/php/progpilot-false-positive.json"

and the json files with reported false positive:

{
  "false_positives": [
    {
	  "vuln_id": "fcfa05bd72416786bcbf09289f64dad31d0afe89145421d42f2023f0198550ad",
	  "vuln_id": "14fad770072acbb70eebdf1aeba31c032d63c6806c2cc94de1c97266d2fea41a"
	}
  ]
}

I tryed with just one:

{
  "false_positives": [
    {
	  "vuln_id": "fcfa05bd72416786bcbf09289f64dad31d0afe89145421d42f2023f0198550ad"
	}
  ]
}

,and like this too:

{
  "false_positives": [
    {
	  "vuln_id": "fcfa05bd72416786bcbf09289f64dad31d0afe89145421d42f2023f0198550ad"
	},
    {
	  "vuln_id": "14fad770072acbb70eebdf1aeba31c032d63c6806c2cc94de1c97266d2fea41a"
	}
  ]
}

But problem are always displayed when i run the phar file : (
The json config is well parsed by progpilot because if the format is not good i got an error message, so the config and the json file is well loaded in progpilot.
Is it a bug or i missed something ? Actually I cannot use it because some false positive are reported and i would like to silent them.

When several vuln_id, which is the good format from my 2 examples ?

Thanks a lot!

@Yivan
Copy link
Author

Yivan commented Feb 17, 2020

For information, i tried with a php file, but the yaml config seems to accept only json config, am i right ?
Another way, is it possible to define the vuln_id array directly in the yaml configuration, i tried but it send a message saying it muset be a file... it seems it is not possible too ?
Best regards.

@Yivan
Copy link
Author

Yivan commented Feb 22, 2020

I found why...
@eric-therond @designsecurity I don't know if this is wanted, but the feature to set false positive is not availables for custom rules. So in this case the only way is to exclude the file or remove the rules, both being not the best.
I don't know if it is a bug or wanted, but if wanted documentation should said this. Actually it is not indicated: https://github.com/designsecurity/progpilot/blob/master/docs/FALSE_POSITIVES.md

I think false positive should work on custom rules too, and hope it could be fixed.

Only tainted rules get the false positive check:

Here are the place to fix, for the other missing type (i imagine just adding an if condition):

Thanks.

eric-therond added a commit that referenced this issue Feb 27, 2020
@eric-therond
Copy link
Collaborator

Thanks for the report and the investigations @Yivan
I have fixed this issue.
v0.7 of progpilot should be released soon.

@Yivan
Copy link
Author

Yivan commented Feb 28, 2020

@eric-therond Thanks a lot for this fast fix.
Juste tested it, and I confirm all is ok!

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

No branches or pull requests

2 participants