stoplightio / spectral-owasp-ruleset

Improve the security of your API by detecting common vulnerabilities as defined by OWASP and enforced with Spectral.
69 stars 11 forks source link

Rules enhancements / api3:2019-define-error-validation #22

Closed Tristan971 closed 1 year ago

Tristan971 commented 1 year ago

After a few comments in https://github.com/apisyouwonthate/style-guide/issues/40, I'll raise a few questions/objections to the current OWASP ruleset. It's not so much a criticism but sharing some perspective we may or may not agree on.


Context

The rule api3:2019-define-error-validation requires that:

Carefully define schemas for all the API responses, including either 400 or 422 responses which describe errors caused by invalid requests.

The HTTP 422 status code is nowhere near ubiquitous enough to be warranted as a mandatory description. Many APIs might decide to not use it, and prefer HTTP 409 Conflict for example in many cases where others use HTTP 422. And/or more granularly split further using other status codes.

Also, I fail to see how this rule relates to security in general? 😕

Current Behavior

As per Context section

Expected Behavior

philsturgeon commented 1 year ago

The rule here is taken from advice on API Security: https://apisecurity.io/encyclopedia/content/owasp/api3-excessive-data-exposure

Screen Shot 2022-12-29 at 11 49 09 AM

Unless there is a bug, 422 is not enforced. The rule says either 400 or 422 should be present, but does not take an opinion on which you prefer to use.

Using 409 Conflict instead of 400 or 422 for validation issues is probably misusing the 409 code, but that's another discussion for elsewhere on the internet.

Tristan971 commented 1 year ago

The rule says either 400 or 422 should be present, but does not take an opinion on which you prefer to use.

Fair enough

Using 409 Conflict instead of 400 or 422 for validation issues is probably misusing the 409 code, but that's another discussion for elsewhere on the internet.

Don't think debating this here is relevant either, but do you perhaps have any pointer to a spec/document for the basis of your opinion on that? Since all I can find suggests that 409 is very much valid (at least in the cases we do use it).

philsturgeon commented 1 year ago

The spec is a great place to look. 400 is "something is wrong with the request" and 409 is more about conflicts with the state of your request against the state on the service, which is a long way from validity. This rule is saying there must be a response explaining invalid requests, but once you've passed validity then checking for conflicts is a good step 2.

https://www.rfc-editor.org/rfc/rfc7231#section-6.5.8

409 Conflict

The 409 (Conflict) status code indicates that the request could not be completed due to a conflict with the current state of the target resource. This code is used in situations where the user might be able to resolve the conflict and resubmit the request. The server SHOULD generate a payload that includes enough information for a user to recognize the source of the conflict.

Conflicts are most likely to occur in response to a PUT request. For example, if versioning were being used and the representation being PUT included changes to a resource that conflict with those made by an earlier (third-party) request, the origin server might use a 409 response to indicate that it can't complete the request. In this case, the response representation would likely contain information useful for merging the differences based on the revision history.