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

Scanner Endpoint ❤️ vuln.json #164

Closed
bkimminich opened this issue Aug 3, 2020 · 5 comments
Closed

Scanner Endpoint ❤️ vuln.json #164

bkimminich opened this issue Aug 3, 2020 · 5 comments

Comments

@bkimminich
Copy link

Hi! I really like your idea of having a kind of manifest of contained vulns in the application. There was actually an attempt to define a "standard" for this driven by https://github.com/OWASP/OWASP-VWAD in 2018 with @psiinon and others but it was dropped due to lack of interest/time/adoption rate/... at some point.

I dug up the artifacts of the __vulns.json proposal from the repo:

The idea back then was, to have a very simple definition file that only maps some kind of flag (could be a URL part or something else that would probably show up in a scanner's finding or report) with a type of vulnerability where we defined a very limited list of things that scanners might be realistically able to find. At least I think that was the rationale...

Now with +2,5 years of experience with the https://github.com/bkimminich/juice-shop project, I would probably do some things a bit different, like rather specifying a known vuln identifier (OWASP-___ or CWE-_____ etc.) than an arbitrary key. Again, something that could easily be found in a scanner's report, so checking it's match rate would be relatively easy.

What I would stick to is having a file (JSON or YAML) in the repo rather than an endpoint to provide this information to the scanner. It can also be retrieved via HTTP-GET from GitHub but it would not spoiler all vulns too easily for human users of the vulnapp.

If you're interested in working out a shared specification, I would consider adding such a file to the Juice Shop as well. And maybe we could motivate others to do the same then.

@preetkaran20
Copy link
Member

preetkaran20 commented Aug 3, 2020

Hi @bkimminich,

I think the idea of having well known vulnerability identifiers (OWASP- or CWE- etc) is amazing and that helps in reducing the mapping of arbitrary keys between systems like ZAP and VulnerableApp/Juice Shop.

Regarding not exposing this information using an endpoint, i think /scanner endpoint only exposes the parameter name, its location in request, the vulnerability types and sample values (which is useful in cases like JWT as an input has a format then scanner can know the format of input for active scanning) etc. however the user of the application still need to find the payload which exposes the vulnerability. So i think it might not be a spoiler for all the vulns for the users (However it certainly exposes some information if we have built a miscellaneous vulnerability level) Please correct me if i am missing something.

Sure it would be great to work on the specification. Actually i was talking with @psiinon a week back about this and i think if we make such a specification then not only for vulnerable apps but any application which wants to effectively use the scanners like ZAP/Burp, can expose the information (may be in testing systems) and scanner's can use this information to find the vulnerabilities.

thanks,
Karan

@preetkaran20 preetkaran20 pinned this issue Aug 3, 2020
@bkimminich
Copy link
Author

If I understand you correctly, then the goal of /scanner is to help scanners to use the right active scanning method or payload, whereas __vuln.json was meant more for helping to check for false positives and false negatives after the scan, by crawling the report for certain "flags" that would indicate if a vuln was found or missed. Assuming a vuln app specifies all its (realistically detectable) vulns in this way, everything not found from the list is false negative and everything found on top is false positive. This would allow scanner comparison with some reliability.

I guess that's also why I assume most entries in such a spec file are spoilers to humans, because they contain parts of the expected finding or URL or similar.

@preetkaran20
Copy link
Member

preetkaran20 commented Aug 4, 2020

Hi @bkimminich ,

Oh, got your point.
Yes you are right this is helpful for scanners to use the right active scanning method, payload format etc but this information can also be useful for scanners to find false positives/negatives etc as /scanner endpoint provides the information about the vulnerability exposed from the endpoint hence if Alerts from ZAP/Burp are for the same vulnerability from that endpoint then we know that ZAP/Burp are able to find the vulnerability and if doesn't then we need to improve them and if they have given other vulnerabilities too then it can be a case of false positive etc.

Actually IMO, from scanner perspective finding the flag can be little tricky as scanners need to know about the response structure returned from the API (vulnerable endpoint) to identify the flag etc. Please correct me if i am missing something.

Also i think, mostly flag concept is for the vulnerable applications and i looked at /scanner endpoint (except vulnerability type) from the perspective that it will be exposed by any application to effectively use scanners.

What do you think ?

thanks,
Karan

@preetkaran20
Copy link
Member

Hi @bkimminich ,

I am thinking of adding CWE identifier along with custom vulnerability identifiers. Also please let me know if you have some other suggestions related to /scanner endpoint.

thanks,
Karan

@bkimminich
Copy link
Author

CWE would be as cool to have for your /scanner as for my vuln.json approach, sure!

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