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 api4:2019-rate-limit states:

All 2XX and 4XX responses should define rate limiting headers.

There is a crucial distinction here between different goals a service might have to implement rate-limiting. And in many cases it is absolutely not appropriate to return rate-limit related headers. Here's a few categories I have in mind:

Preventing a malicious actor, on their account or a compromised one, from doing extensive damage to the website before the administrators are able to react

It isn't clear whether one would actually want such an actor to be able to perfectly know the limits imposed on them. The general guidance would usually rather be to have them set in a way that "you won't hit it unless you're doing something extremely wrong".

Yeah it sucks for false-positives, but it's the administrator's job to refine such that those don't happen.

And depending on the case and sensitivity of the operations, maybe it's fine to add headers. But it's more of a guidance in such cases than an error-level failure.

Stop DoS (and related) attacks

This one is a more specific version of the previous one, where the means an administrator has to respond to attacks is severely limited:

• You might not have the compute power to generate responses fast enough to avoid an outage
• You might not have the bandwidth to generate responses without causing an outage

In practice, for a service under severe threat of DoS attacks, you need to constantly adjust and add some forms of matching way before the request even reaches your API so that you entirely IP ban the end user as soon as possible, entirely dropping their connections without any further form of processing.

And obviously, you absolutely cannot have any of this be public knowledge.

In the end, even if you wanted to be clear about the limits at which you consider traffic to be a DoS attack, unless you're the likes of Google, Cloudflare, and other megacorps with unlimited budget, you most likely couldn't even compute accurate responses in real-time due to the time and performance constraints.

Business logic reasons to limit the rate of use of certain endpoints

That is a fine use-case of these headers.

Current Behavior

As per Context section

Expected Behavior

• Make the rule a warning at most for endpoints in general
• Make it instead mandate an HTTP 429 response (at error-level rather than the current warning one) description perhaps instead if you believe it to be necessary

Current Behavior

If my .spectral.yml looks like

extends: ["https://unpkg.com/@stoplight/spectral-owasp-ruleset"]

as you write in your documentation I get following error.

$ spectral lint .spectral.yml https://raw.githubusercontent.com/OAI/OpenAPI-Specification/main/examples/v3.0/petstore.yaml --verbose
Error running Spectral!
Error #1: Invalid ruleset provided
          at assertRuleset         …idation/index.js:14  throw new Error('In…
          at read                  …or/dist/index.js:20  (0, validation_1.as…
          at processTicksAndReje…  …cess/task_queues:96
          at async processExtend   …rmers/extends.js:22  return await (0, __…
          at                       …rmers/extends.js:40

The same happens if I use your Docker image.

If I install @stoplight/spectral-owasp-ruleset package and my .spectral.yml looks like

extends: ["@stoplight/spectral-owasp-ruleset"]

it works as expected.

Environment

• Version used: 6.5.1
• Environment name and version (e.g. Chrome 39, node.js 5.4): Node.js v16.13.2
• Operating System and version (desktop or mobile): Windows 10

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

• The lack of an HTTP 422 description shouldn't be part of the rule altogether

User Story Description

As a USER,

I want to reference a YAML ruleset,

so I can test a specification without installing a package or executing a js file.

Acceptance Criteria

• [ ] a YAML version of the ruleset is published

Sprint Ready Checklist

• [ ] Acceptance criteria defined
• [ ] Team understands acceptance criteria
• [ ] Team has defined solution / steps to satisfy acceptance criteria
• [ ] Acceptance criteria is verifiable / testable
• [ ] External / 3rd Party dependencies identified

{
    name: "valid case: oas3.1",
    document: {
      openapi: "3.1.0",
      info: { version: "1.0" },
      components: {
        schemas: {
          type: {
            type: "string",
            maxLength: 99,
          },
        },
      },
    },
    errors: [],
  },

This case fails with an error. It shouldn't

User Story Description

As an API designer I want to be sure that I'm securing my API by precisely defining the types, and patterns you will accept in requests at design time.

Acceptance Criteria

• [x] Define min/max length for integer
• [x] Define format for integer
• [x] Define max length for string
• [x] Define pattern for string
• [x] Severity: Error

More information: https://apisecurity.io/encyclopedia/content/owasp/api7-security-misconfiguration

:rotating_light: The automated release from the main branch failed. :rotating_light:

I recommend you give this issue a high priority, so other packages depending on you can benefit from your bug fixes and new features again.

You can find below the list of errors reported by semantic-release. Each one of them has to be resolved in order to automatically publish your package. I’m sure you can fix this 💪.

Errors are usually caused by a misconfiguration or an authentication problem. With each error reported below you will find explanation and guidance to help you to resolve it.

Once all the errors are resolved, semantic-release will release your package the next time you push a commit to the main branch. You can also manually restart the failed CI job that runs semantic-release.

If you are not sure how to resolve this, here are some links that can help you:

• Usage documentation
• Frequently Asked Questions
• Support channels

If those don’t help, or if this issue is reporting something you think isn’t right, you can always ask the humans behind semantic-release.

No npm token specified.

An npm token must be created and set in the NPM_TOKEN environment variable on your CI environment.

Please make sure to create an npm token and to set it in the NPM_TOKEN environment variable on your CI environment. The token must allow to publish to the registry https://registry.npmjs.org/.

Good luck with your project ✨

Your semantic-release bot :package::rocket:

Renaming 'owasp:api3:2019-rate-limit', 'owasp:api3:2019-rate-limit-retry-after' and 'owasp:api3:2019-rate-limit-responses-429' to begin with 'owasp:api4...' to match the API4:2019 OWASP code.

Motivation and Context

Looks like there was a typo in the naming of 3 of the rules. Perhaps should have been owasp:api4.. instead of owasp:api3... ?

#InsertIssueNumberHere

Description

Renamed rule and tests

How Has This Been Tested?

Ran unit tests

Screenshot(s)/recordings(s)

Types of changes

• [ x] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• [x ] This PR's code follows as closely as possible the coding style/guidelines of this project.
• [ ] I have added error reporting and followed the error reporting guidelines.
• [ ] I have added event tracking and followed the event tracking guidelines.
• [ x] I have updated any relevant documentation accordingly to reflect this PR's changes.
• [ ] I have added automated tests (unit/integration/e2e/other) to cover my changes.
• [ x] All new and existing tests pass locally (excluding flaky CI tests).

User Story

As an API Designer I want to be informed if my definition is missing a 401 response So that my consumers know they will receive a 401 if they fail to authenticate

Acceptance Criteria

For all operations on all paths An HTTP 401 response is defined

Sprint Ready Checklist

• [ ] Acceptance criteria defined
• [ ] Team understands acceptance criteria
• [ ] Team has defined solution / steps to satisfy acceptance criteria
• [ ] Acceptance criteria is verifiable / testable
• [ ] External / 3rd Party dependencies identified

User Story Description

As an API designer I should explain how my API handles "too many" requests, to improve security and clarity for the API consumers looking at the docs.

Acceptance Criteria

• [x] MUST have a Response with HTTP Status Code 429.
• [x] SHOULD have a Retry-After on the 429 Response.
• [x] MUST have rate limit headers defined, either:
  • [x] IETF Draft RateLimt Headers (case insensitive).
  • [x] X-Rate-Limit headers (case insensitive).

Trying to add check security, which will make sure there is either global security applied or operation level security, for every operation. Sadly I cannot work out how to get it going with this slick TypeScript format, as the docs don't cover that and my guesswork is leading me to:

spectral lint ../protect-earth-api/api/openapi.yaml --ruleset=src/ruleset.ts --verbose
Error running Spectral!
Error #1: Could not resolve './functions/checkSecurity' from src/ruleset.ts
          at error            …hared/rollup.js:198  base = Object.assig…
          at handleResolveId  …red/rollup.js:22508  return error(errUnr…
          at                  …red/rollup.js:22471  this.handleResolveI…

Any tips welcome.

the functionality in the repo was contradicting itself, saying it had to be false but also another rule allowed it to be an object. OAS3.0 is additionalPorperties: false by default, and OAS3.1 is additionalProperties: true by default. It seems like we should address that, by making OAS3.1 users use additionalProperties, and saying it either needs to be false or an object with maxProperties set?

Motivation and Context

#InsertIssueNumberHere

Description

How Has This Been Tested?

Screenshot(s)/recordings(s)

Types of changes

• [ ] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• [ ] This PR's code follows as closely as possible the coding style/guidelines of this project.
• [ ] I have added error reporting and followed the error reporting guidelines.
• [ ] I have added event tracking and followed the event tracking guidelines.
• [ ] I have updated any relevant documentation accordingly to reflect this PR's changes.
• [ ] I have added automated tests (unit/integration/e2e/other) to cover my changes.
• [ ] All new and existing tests pass locally (excluding flaky CI tests).

In API5:2019 — Broken function level authorization of OWASP API Security they talk about a bunch of things which are pretty tricky to detect in OpenAPI, but not entriely impossible.

Context

Is the API Vulnerable?

The best way to find broken function level authorization issues is to perform deep analysis of the authorization mechanism, while keeping in mind the user hierarchy, different roles or groups in the application, and asking the following questions:

• Can a regular user access administrative endpoints?
• Can a user perform sensitive actions (e.g., creation, modification, or erasure) that they should not have access to by simply changing the HTTP method (e.g., from GET to DELETE)?
• Can a user from group X access a function that should be exposed only to users from group Y, by simply guessing the endpoint URL and parameters (e.g., /api/v1/users/export_all)?

Don’t assume that an API endpoint is regular or administrative only based on the URL path.

While developers might choose to expose most of the administrative endpoints under a specific relative path, like api/admins, it’s very common to find these administrative endpoints under other relative paths together with regular endpoints, like api/users.

We can't guarentee an operation isnt admin related (as it says dont assume on path alone) but we can catch common case where the path does have /admin/ and warn about that.

Current Behavior

Currently no efforts are made for anything in API5:2019.

Expected Behavior

Any path containing /admin/ should error if the securityScheme is the same for non /admin/ paths.

Possible Solution(s)

Probably needs a custom function to check what securitySchemes exist. They either need to be a different securityScheme.Foo or securityScheme.Bar.

It's possible that both could be securityScheme.Foo if type: oauth2 and they have different scopes.

  "securitySchemes": {
    "api_key": {
      "type": "apiKey",
      "name": "api_key",
      "in": "header"
    },
    "petstore_auth": {
      "type": "oauth2",
      "flows": {
        "implicit": {
          "authorizationUrl": "https://example.org/api/oauth/dialog",
          "scopes": {
            "write:pets": "modify pets in your account",
            "read:pets": "read your pets"
          }
        }
      }
    }
  }

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 api4:2019-string-restricted requires that:

Schema of type string must specify a format or pattern. To avoid unexpected values being sent or leaked, ensure that strings have either a format or a RegEx pattern. This can be done using format or pattern.

This is rather questionable in a lot of cases where freeform input might be meant (and thus anything short of a .+ allowed pattern will be incorrect).

Also, from a pure security standpoint, user-defined strings should instead always be considered as random text with 0 particular technical meaning. Otherwise you get all kinds of vulnerabilities like XSS, SQLi, etc.

The use of patterns is otherwise okay for pure business rules without particularly high risks if a malicious actor finds a bypass of some sort.

Current Behavior

As per Context section

Expected Behavior

• At least make it a warning rather than an error
• Maybe find a way to send users over to some open-source WAF ruleset like https://github.com/coreruleset/coreruleset
• If you insist on having it, make it a warning, and make it an error to have a pattern that isn't bounded (ie ^pattern$) since that's by far the number one bypass reason of these and much more of a threat due to the false sense of security an administrator might have otherwise

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 api1:2019-no-numeric-ids more or less enforces usage of UUIDs as public resource identifiers, to avoid unsecured APIs to allow malicious users to guess URIs of resources they shouldn't access.

I would oppose the following to the current rule:

1. UUIDs' randomness isn't security, as they're not a cryptographically secure source of randomness
2. Numeric identifiers aren't any more insecure, so long as they are behind proper permission checks

Current Behavior

As per Context section

Expected Behavior

1. Allow numeric resource identifiers, so long as they're gated behind an authentication schema
2. Optionally: Allow some way to flag a resource endpoint as explicitly public and meant to be traversable, such that numeric identifiers aren't unnecessarily flagged (imagine something like /announcements/1, /announcements/2, /announcements/3 for a public announcement system)

User Story Description

As an API developer I want to be sure that I'm not leaking backtraces, so if I design an API that says I will it should error, and if I omit this error contract testing will catch it. Either way I then know I am not leaking backtraces.

Acceptance Criteria

• [ ] Supports backtrace detection for Java, PHP, Ruby, Python, and NodeJS
• [ ] Assume its JSON and its probably got some awkwardly encoded \n\n strings.
• [ ] Severity: Error

More information: https://apisecurity.io/encyclopedia/content/owasp/api7-security-misconfiguration

User Story Description

As an API Designer I should probably create a shared CORS header and apply it to all my responses because I always forget to add CORS, and it would be nice if Spectral could remind me to spec it, so that Prism Proxy will remind me to actually code it in. Otherwise this will definitely go online without CORS.

Acceptance Criteria

• [ ] MUST have Access-Control-Allow-Origin defined regardless of value.
• [ ] The message points people to good CORS documentation: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Origin and https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS

More information: https://apisecurity.io/encyclopedia/content/owasp/api7-security-misconfiguration