[HN Gopher] I've fuzzed parts of the GitHub API. Here are my fin...
___________________________________________________________________
I've fuzzed parts of the GitHub API. Here are my findings (2020)
Author : ludovicianul
Score : 72 points
Date : 2022-11-30 13:24 UTC (9 hours ago)
(HTM) web link (ludovicianul.github.io)
(TXT) w3m dump (ludovicianul.github.io)
| dtech wrote:
| > The InvalidValuesInEnumsFieldsFuzzer will send invalid values
| in enum fields. It expects a validation error in return. The
| GitHub API does not seem to reject invalid values, but rather
| convert them to a default value and respond successfully to the
| request. This is in contradiction with the OWASP recommendation
| around strong input validation and data type enforcing.
|
| Doing this is incredibly good practice for compatibility, because
| otherwise you can never add or remove enum values because older
| clients will break on the unknown values. I also fail to see how
| it violates the recommendation, invalid data doesn't enter the
| system.
|
| Imagine all webservers and browsers would need to be updated for
| every new HTTP header or status code is specified or web pages
| don't work anymore...
| ludovicianul wrote:
| As long as an API rejects invalid values if they are not part
| of an agreed list (but not enum), the same logic should apply
| for fields explicitly marked as enums. HTTP headers are not
| meant to be a limited set. As for the HTTP methods, it's a
| clear recommendation from OWASP to explicitly reject methods
| not supported by your API. It's like converting everything not
| known to POST...
| plebb0t wrote:
| Ah yes, ISO 8061, Alpine ski-bindings -- Selection of release
| torques values.
|
| https://www.iso.org/standard/75609.html
|
| Nice article btw.
| rfdonnelly wrote:
| I submitted a PR
| https://github.com/ludovicianul/ludovicianul.github.io/pull/...
| ludovicianul wrote:
| Thanks!
| jwilk wrote:
| Quite popular misspelling:
|
| https://codesearch.debian.net/search?q=%28%3Fi%29%5Cbiso%5B+...
| londons_explore wrote:
| I think the real conclusion here is... No substantial security
| issues were discovered. There are a few things github could
| change to better follow security best practices to help avoid
| future bugs.
| dropofwill wrote:
| I read it more as an example of how to use CATS to do this sort
| of fuzzing.
| anentropic wrote:
| Yes, CATS looks great, hadn't heard of it before!
| londons_explore wrote:
| Doing this sort of thing to a live webservice is a good way to
| get a perma-ban...
| ludovicianul wrote:
| Author here. There are no major security issues and it's also
| compliant with the API rate limiting restrictions.
| masklinn wrote:
| Including the new secondary rate limits? They're quite
| annoying.
| bobleeswagger wrote:
| It's a fine line, if Github _wanted_ to I 'm sure they could
| write up a convincing cease and desist. I think they spoke up
| about it because your article gives the impression that this
| sort of thing (fuzzing APIs) is okay, which I wouldn't
| exactly say is positive.
| ludovicianul wrote:
| True. This is why I run it only for one path. I also run it
| against Kubernetes API and some of the payloads resulted in
| timeouts which I submitted to the security team.
| deluarseo wrote:
| binarysneaker wrote:
| GitHub performs fuzzing as part of their testing, so I'm not
| surprised there aren't any significant findings. I _am_ surprised
| this is on hackernews two years later though.
| sigmonsays wrote:
| the results of the fuzzing is overall very meh, but CATS looks
| like an interesting tool.
| aszen wrote:
| The cats tool looks interesting I'm curious how it handles
| fuzzing id columns, which would need a reference to another
| resource to actually test them. I would like a tool which can
| take insertion order of the api and start inserting things at
| random based on a spec to test it.
| ludovicianul wrote:
| Can you please detail a bit?
|
| Currently, there is some support for resource correlation.
| Especially when fuzzing DELETE, it will identify a successful
| POST based on conventions and use that as an identifier. Also
| checking that DELETEd resources are not available anymore.
| brap wrote:
| Would any of these errors be present if they used GraphQL instead
| of REST? Obviously the ones with headers can still happen, but it
| looks like everything else is related to schema enforcement
| ludovicianul wrote:
| It's only OpenAPI for now with limited support for request
| templates.
___________________________________________________________________
(page generated 2022-11-30 23:02 UTC)