search

OWASP / ASVS

Application Security Verification Standard
Creative Commons Attribution Share Alike 4.0 International
2.74k stars 668 forks source link

Discussion: Too much and/or sensitive information in (mostly API) responses #934

Closed elarlang closed 3 weeks ago

elarlang commented 3 years ago

Had it in my todo-list but as WSTG has this topic under discussion I make link/placeholder here as well: https://github.com/OWASP/wstg/issues/727 by @pinkLagoon

Problem: in response from an application there is too much information which may be also sensitive. Happens, when developers are JSON-encoding entire dataset or object and give it for HTTP response.

Risks:

Lists in the beginning are small, but if someone "is testing" with every field with max-length value (text fields getting problematic quite fast) - like few hundred MB to download by API most likely is causing some problems for serving the content or handling the content on the client side - availability loss.

Proposal:

elarlang commented 3 years ago

Category and requirement text - probably direction should be current requirement 8.1.3:

V8.1.3 Verify the application minimizes the number of parameters in a request, such as hidden fields, Ajax variables, cookies and header values.

jmanico commented 3 years ago

I do not think it's enough to say "just minimize parameters, etc". What you're really asking is that we only include data in JSON (1) that the user has access to and (2) that the application actually needs.

And I'm confused, the original post is about responses, but the last comment was about requests?

elarlang commented 3 years ago

8.1.3 was taken as an example, where (to what category) to place it in ASVS. Requirement itself is for response.

jmanico commented 3 years ago

I personally see this as a pure access control requirement. And it's a core principle. I suggest 4.1 or 4.3

elarlang commented 3 years ago

Yes, part of that is authorization problem and initially I was thinking about this problem more like V4.* problem.

I have seen examples where frontend needs list of titles, but entire entity datasets are provided ended up responses with size of many MB.

jmanico commented 3 years ago

I am with you 100% and think this is a great idea for a new requirement! Even if the user has access to the data, it should not be sent to the client unless the user has access AND the app actually needs that data! :)

csfreak92 commented 3 years ago

+1 on this requirement, I actually saw a badly designed/implemented app where the whole API is sending everything to the front-end to make the application's response "faster" but it ended up including a bunch of sensitive information that should never be there in the first place.

cmlh commented 3 years ago

Should "sensitive" be inserted into ASVS Level 1 to mirror the use of the term "sensitive" within the description of ASVS Level 2?

elarlang commented 2 years ago

todo, check requirement 13.1.3 as potential duplicate or place to merge, if we can find some solution here:

V13.1.3 Verify API URLs do not expose sensitive information, such as the API key, session tokens etc.

jmanico commented 2 years ago

This is not a problem with API’s it's a problem with any Url due to referrer headers, logs, browser history and similar.

elarlang commented 2 years ago

This is not a problem with API’s it's a problem with any Url due to referrer headers, logs, browser history and similar.

this is what you described, is covered by 8.3.1

V8.3.1 Verify that sensitive data is sent to the server in the HTTP message body or headers, and that query string parameters from any HTTP verb do not contain sensitive data.

the problem is not the method how sensitive data is sent to client, the problem is the fact that sensitive data is exposed to the client.

elarlang commented 2 years ago

Access control part maybe can be merged to current 1.4.5 (this one need to be moved to V4): Verify that attribute or feature-based access control is used whereby the code checks the user's authorization for a feature/data item rather than just their role. Permissions should still be allocated using roles.

tghosth commented 2 years ago

Which of the following problem options are we suggesting that this is:

  1. An Authorization problem where the user is receiving data in the API that they should not have permissions to access?
  2. An efficiency issue where data is being returned that the user has permissions to but is more data than they need for the current functionality?
tghosth commented 1 year ago

V4 Access Control at the JSON attribute level, i.e. you can get the user's firstname and last name but not their password hash, even if it is in the same conceptual object/entity.

Business Logic - Verify that data pagination is enforced at the server side so that the amount of data that can be accessed at once is limited, matching the limit enforced at the front-end.

Too many fields (which the user is permitted to access) seems a little too specific and niche and probably not a serious issue.

@set-reminder 5 weeks @tghosth to look at this.

octo-reminder[bot] commented 1 year ago

⏰ Reminder Monday, January 16, 2023 12:00 AM (GMT+01:00)

@tghosth to look at this.

octo-reminder[bot] commented 1 year ago

🔔 @tghosth

@tghosth to look at this.

tghosth commented 1 year ago

@elarlang should we work on this now or wait for the full access control reorg?

elarlang commented 1 year ago

@elarlang should we work on this now or wait for the full access control reorg?

We can work on this now but I prefer we take one topic and cover it all at once. Otherwise at the moment I should put my time (again) to investigate all the background and history for give answer and then it waits again few months... and we need to start again from the start.

elarlang commented 2 months ago

Problem 1 - the application returns data, the user does not need to see - unintended (potentially sensitive) information leakage

It is not an authorization decision problem - a user may have access to read the object (e. g. user), but not the served attribute of that object (e. g. password hash)

Needs a separate requirement. The requirement can be in V4 or V8.

Problem 2 - possible to ask too many rows to the output - bypassing anti-automation, resource-demanding response building (potential DoS vector)

In general, it's a missing (business logic) input validation problem and seems to be covered by:

11.1.3 Verify that business logic limits and validations are implemented as per the application's documentation.

To have a separate requirement to say "validate the rows-per-page parameter" seems a bit too detailed, but at the same time, it is a really widespread problem (based on my experience).

Problem 3 - too much (not needed) data in the output - resource-demanding response buildng (potential DoS vector)

We have the requirement:

11.2.2 Verify that the application has anti-automation controls to protect against excessive calls to application functionality which could result in mass data exfiltration, junk data creation, resource quota exhaustion, rate limit breaches, out-of-band communication flooding, denial of service, overuse of an expensive resource, etc.

Although pointed out problem is not how to avoid the call to resource-demanding response building, but how to avoid that such unnecessary resource-wasting functionality as potential Denial-of-Service vectors exist.

Do we consider the problem covered by 11.2.2 or is it worth separate mention somehow?

jmanico commented 2 months ago

I read this thread carefully. I think the access control issue is really important and should go into the access control re-org.

The performance issue to too much data is a DOS issue that is really easy to fix. I don't think DOS is a really big deal in most apps, even though it is in some. I would suggest we focus on the access control issue and drop DOS issues for the sake of brevity.

I think the access control issues is super critical. The DOS issues described is rarely very critical (but can be on a rare occasion).

tghosth commented 2 months ago

Problem 1

Does the 4th bullet of this comment solve this @elarlang?

Problem 2

Yeah I think that is covered by 11.1.3 and you could also argue that it is covered by the updated 5.1.3.

Problem 3

Feels like this is covered by 11.2.2 and maybe also the updated 5.1.3.

elarlang commented 1 month ago

Does the 4th bullet https://github.com/OWASP/ASVS/issues/2065#issuecomment-2336743995 solve this @elarlang?

The linked requirement is added to V1.4 Access Control Architecture via #2065 / #2123

# Description L1 L2 L3 CWE
1.4.7 [ADDED] Verify that access control documentation defines the rules for access control decision-making, specifying user and subject attributes, resource attributes, and relevant environmental factors involved in the process. ✓ ✓ ✓

I would say that is not cover the problem statement:

Problem 1 - the application returns data, the user does not need to see - unintended (potentially sensitive) information leakage

1.4.7 asks to describe the attributes, based on what you going to make access control decisions. "Problem 1" needs to point out attributes, that you can or can not see/access.

tghosth commented 1 month ago

Ok well hopefully @EnigmaRosa will see this as part of here work on V4, and make sure that the access control requirements cover all the various dimensions that I mentioned here: https://github.com/OWASP/ASVS/issues/2065#issuecomment-2336743995

  • Is the user allowed to interact with this particular entity type (e.g. orders in an eCommerce system)
  • Is the user allowed to perform this particular action on this entity type (e.g. can they create an order or only view an order)
  • Is the user allowed to interact with this particular instance of this entity type (e.g. can they only see or modify orders for their department but not other departments)
  • Is the user allowed to interact with these particular fields of this particular entity type (e.g. they can view order details but they are not allowed to see the standard discount amount)
elarlang commented 1 month ago

As I pointed out previously (https://github.com/OWASP/ASVS/issues/934#issuecomment-2325740099) - if we talk about V4 as "access control decisions", then the requirement is not a V4 requirement. But I'm not sure there is better place for that.

A seed for future development:

Verify that the application returns only the data the user has permission to see. For example, the API response does not return a full object with attributes that contain values the user has no permission to see although having permission to see the data object.

EnigmaRosa commented 1 month ago

I would think that the language in 1.4.7, specifically "documentation defines the rules for access controls" meets the dimensions @tghosth pointed out. I don't know if those points can go unsaid, if we should outline them in guidance, or create another verification requirement surrounding the dimensions (which I feel would be redundant).

With regards to the specific concern @elarlang posed, I like the requirement he proposed as a 4.1.x or 4.2.x

tghosth commented 3 weeks ago

Ok so let's leave the dimensions in section guidance for either 1.4.x or 4.x.x. Can you open a PR for that @EnigmaRosa?

@elarlang I'd be comfortable for your proposed requirement to go into 4.1.x or 4.2.x as Shanni suggested. Do you agree?

elarlang commented 3 weeks ago

@elarlang I'd be comfortable for your proposed requirement to go into 4.1.x or 4.2.x as Shanni suggested. Do you agree?

Well, it expects that V4.1 and V4.2 have clear definition, what kind of requirements belong there (#2139).

By principle, returning too much data is not about an authorization decision, but just an incorrect behavior from the application. It is pretty much the same problem as mass parameter assignment (ASVS v4.0.3-5.1.2) that we placed to "V10.4 Defensive Coding" as V10.4.4.

EnigmaRosa commented 3 weeks ago

Should section guidance go right before the verification requirements?

@elarlang I didn't realize we now have a defensive coding section, your requirement might fit in there more.

tghosth commented 3 weeks ago

Should section guidance go right before the verification requirements?

See my comment in @2139 https://github.com/OWASP/ASVS/issues/2139#issuecomment-2415018916

tghosth commented 3 weeks ago

@elarlang can you PR your suggestion into V10.4 as I think that is a sensible suggestion

elarlang commented 3 weeks ago

Goes in via #2155 , changed "see" to "access" from previous proposal.

# Description L1 L2 L3 CWE
10.4.5 [ADDED] Verify that the application returns only the data the user has permission to access. For example, the API response does not return a full object with attributes that contain values the user has no permission to access although having permission to access the data object. ✓ ✓ ✓