API Security 2023

[philsturgeon]

Updating the Spectral OWASP ruleset to contain as much of the OWASP API Security Top 10 (2023) as possible with a tool that focuses on scanning the interface.

Motivation and Context

Times change, the world gets more complicated, and OWASP like to move things around. 😋

• Fixes New Rule: Require CORS (OWASP API8) #5
• Fixes Update Spectral OWASP Ruleset to Cover OWASP 2023 Standards #47
• Fixes Rules enhancements / api4:2019-string-restricted #24
• Fixes owasp:api7:2019-security-hosts-https-oas3 has incorrect validation per OAS 3.x #27
• Fixes API9:2023 - Improper Inventory Management #52
• Includes Removes duplicated test case for Integer Limit Rule #45
• Includes Allow schemas to use unevaluatedProperties: false instead of additionalProperties: false #49
• Closes Fixes #49 by adding rules for JSON Schema unevaluatedProperties keyword #50
• Possibly solves Rules enhancements / api1:2019-no-numeric-ids #21
• Fixes API7:2023 - Check for redirect URLs #55
• Fixes New Rule: Check for /admin/ in the path and see if its the same securityScheme as non admin paths. #25

Description

Updating the Spectral ruleset to match the changes, add new rules where possible, and delete things which no longer seem as relevant.

https://owasp.org/API-Security/editions/2023/en/0x00-header/

How Has This Been Tested?

Testing has been done through unit tests for all new rules.

[philsturgeon]

@frankkilcommins fixed all feedback, and upgraded npm dependencies too whilst I'm here.

[mnaumanali94]

@philsturgeon We should probably get these rules as part of this done too:
#5
#3
#12

What do you think?

[mnaumanali94]

For API7:2023 Server Side Request Forgery I was thinking we could enforce that a parameter is named as something like URL i.e e.g., callbackUrl, redirectUri we should
1: flag it for review and/or have a URL format on it + validation pattern

[mnaumanali94]

For API9:2023 Improper Inventory Management
We could add rule(s) to add Good descriptions, version number, contact info e.t.c
I think some of our other rulesets cover that but maybe we can refer them here or recommend using the docs style guide with this 🤔

[mnaumanali94]

I think we can make these JSON paths a bit more specific i.e. target various places that have responses property rather than doing ..responses. This can lead to erroneous messages and be relatively slow.

Don't need to do this in this PR but probably good to create an issue for this 🤔

[philsturgeon]

We need to be a little bit specific because there have been several issues popping up where JSON in examples, where the example JSON might be { responses: { ... } } so I've always tried to point it somewhat in the right direction. I would love somebody better versed with nimma performance to help me understand the right balance of specific and fast but I don't know it insde and out.

[philsturgeon]

For API9:2023 Improper Inventory Management We could add rule(s) to add Good descriptions, version number, contact info e.t.c I think some of our other rulesets cover that but maybe we can refer them here or recommend using the docs style guide with this 🤔

I don't believe anything in OWASP API Security Top 10 has any opinion on this. IT definitely doens't care about contact info or description, and the OpenAPI version number might be a bit of a stretch because that doesn't necessarily mean it's the API version number, and even if it does the problem its describing is having too many API versions hosted seperately and this one API document doesn't have any way to know what other APIs are hosted.

I would be happier leaving those sort of "make good docs" rules to the Documentation ruleset, so we can let people mix and match things in a modular way, and it means people wont be annoyed by the ruleset telling them irrelevant stuff.

[philsturgeon]

For API7:2023 Server Side Request Forgery I was thinking we could enforce that a parameter is named as something like URL i.e e.g., callbackUrl, redirectUri we should 1: flag it for review and/or have a URL format on it + validation pattern

Good reminder. I've made this #55 with a bit more context, as I really wanted to add this but have been a bit stuck with the direction, and if it's useful to even attempt.

[philsturgeon]

@philsturgeon We should probably get these rules as part of this done too: #5 #3 #12

What do you think?

Happy to give #5 a quick go because you're absolutely right, that needs doing.

#12 do we think people are going to have actual back traces in their examples? Perhaps this would happen in some sort of automated OpenAPI learning scenario? I cannot imagine this happening any other way, so I do not know if this is helpful.

#3 This one would be lovely to do, and it's something I got working with LintGPT as it's using AI to figure out if any of the field names seem like PII, but I am pretty concerned about adding checks for every single schema to see if it has any fields from a massive list that might be PII.

If I have to check every response for id, email, password, address, credit card, gender, etc then that'll be slow. Secondly, is that leaking PII, or is that PII right where its meant to be? If I have a user directory and its VPNed off from the rest of the world, that is not leaking PII it's just a system that has PII.

What am I actually looking for here, and how can I implement that in a way which isn't horrendously slow.

[frankkilcommins]

#12 do we think people are going to have actual back traces in their examples? Perhaps this would happen in some sort of automated OpenAPI learning scenario? I cannot imagine this happening any other way, so I do not know if this is helpful.

I think it would indeed be strange to intentionally put such traces etc. into an example. What's the thinking around trying to give some use feedback on an API design description?

Perhaps:

• warn if no error response schemas (already covered)
• warn if error response schema is very wide ( {} )
• warn if not following RFC 9457 (formally RFC 7807)
• warn if leaky properties are defined in error response schemas. For example, properties like log/trace/diagnostics/logDetails/stacktrace/exception (will someone really design like this tho...)

[philsturgeon]

@mnaumanali94 I've added #5 and #55.

I do not believe #3 can be done in a way that is both useful and vaguely performant.

As @frankkilcommins suggests I don't think #12 can be done in a way which is meaningful and sticks to the instructions in the OWASP edition.

Yes, we could say "you have to use error format X or Y" and then hopefully they have contract testing, but thats no different to owasp:api8:2023-define-error-responses-401 or owasp:api8:2023-define-error-responses-500 which already ask you to define the content and that will already pick up on your accidentally leaking weird errors if they dont match the schema defined.

This is more likely to be a check for Prism, where it's noticing a mismatch between OpenAPI and the response from the API implementation, but seeing as Spectral can only look at the design (and people arent going to design unexpected broken backtrace errors into their specs) then I just really don't think there's anything to do here.

[frankkilcommins]

Changes now look good to me. @mnaumanali94 will need to do the formal approval

[mnaumanali94]

Great work Phil!

[mnaumanali94]

@philsturgeon When you get a chance can you please resolve the conflicts and then I think we can go ahead and merge.

[github-actions]

🎉 This PR is included in version 2.0.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀