Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding clarity for 1.5.1 and 5.1.4 (related 5.1.3, 1.8.1) #1552

Open
jmanico opened this issue Feb 17, 2023 · 20 comments
Open

Adding clarity for 1.5.1 and 5.1.4 (related 5.1.3, 1.8.1) #1552

jmanico opened this issue Feb 17, 2023 · 20 comments
Assignees
Labels
4b Major-rework These issues need to be part of a full chapter rework Community wanted We would like feedback from the community to guide our decision otherwise we will progress V5 Temporary label for grouping input validation, sanitization, encoding, escaping related requirements _5.0 - prep This needs to be addressed to prepare 5.0

Comments

@jmanico
Copy link
Member

jmanico commented Feb 17, 2023

Suggest we augment 5.1.4 from:

5.1.4 Verify that structured data is strongly typed and validated against a defined schema including allowed characters, length and pattern (e.g. credit card numbers, e-mail addresses, telephone numbers, or validating that two related fields are reasonable, such as checking that suburb and zip/postcode match). (C5) 20

to:

5.1.4 Verify that structured data (e.g., JSON and XML) is strongly typed and validated against a defined schema, including allowed characters, length, and patterns such as regular expressions for credit card numbers, e-mail addresses, and telephone numbers. (C5) 20
@elarlang
Copy link
Collaborator

I would use JSON and XML schema validation separately, 5.1.4 can send more clear message as "allow-listed pattern" for validation.

JSON and XML schemas are covered with requirements:

# Description L1 L2 L3 CWE
13.2.2 Verify that JSON schema validation is in place and verified before accepting input. 20
13.3.1 Verify that XSD schema validation takes place to ensure a properly formed XML document, followed by validation of each input field before any processing of that data takes place. 20

@jmanico
Copy link
Member Author

jmanico commented Feb 17, 2023

Thanks @elarlang - would you suggest we delete 5.1.4 as is then? It seems really JSON/XML specific.

@elarlang
Copy link
Collaborator

No, I don't suggest that.

With requirement 1.5.1 must be defined, how some data must be validated and with 5.1.4 analyzt must follow the ruleset from 1.5.1 and verify that. The word schema maybe was directing you to JSON and XML fields, but otherwise I think those requirements are in correct place.

# Description L1 L2 L3 CWE
1.5.1 Verify that input and output requirements clearly define how to handle and process data based on type, content, and applicable laws, regulations, and other policy compliance. 1029

You opened separate issues for 1.5.1 (#1543, #1546)

@jmanico
Copy link
Member Author

jmanico commented Feb 17, 2023

Yes, schema implies JSON and XML schema and is throwing me off for that requirement, its confusing and need fixing IMO.

@mgargiullo
Copy link

While I see how you can see where schema may imply JSON or XML, it really is using the definition meaning "identified or specified pattern". Unless we have another requirement that addresses pattern specific data fields like email address, SSN, phone number, etc., I'd say we leave 5.1.4 as is. If the word "schema" is confusing for many, maybe we swap it for something like "specified patterns".

@elarlang elarlang self-assigned this Feb 22, 2023
@elarlang elarlang added the 3) awaiting proposal There is some discussion in issue and reach to some results but it's not concluded with clear propos label Feb 22, 2023
@tghosth
Copy link
Collaborator

tghosth commented Mar 14, 2023

Need to make sure we don't overlap with 1.8.1:

1.8.1 [MODIFIED, MERGED FROM 8.3.4, LEVEL L2 > L1] Verify that all sensitive data created and processed by the application has been identified and classified into protection levels, and ensure that a policy is in place on how to deal with sensitive data. 213

@tghosth
Copy link
Collaborator

tghosth commented Mar 14, 2023

Any further comments?

@tghosth tghosth added the Community wanted We would like feedback from the community to guide our decision otherwise we will progress label Mar 14, 2023
@elarlang
Copy link
Collaborator

If 1.5.1 is pre-condition for 5.1.4, then 1.5.1 should be also level 1...

@elarlang
Copy link
Collaborator

And second thing - we need to take also 5.1.3 into the game and make clear separation, which requirement is meant for what.

@elarlang elarlang added 1) Discussion ongoing Issue is opened and assigned but no clear proposal yet and removed 3) awaiting proposal There is some discussion in issue and reach to some results but it's not concluded with clear propos labels Mar 20, 2023
@elarlang elarlang added the V5 Temporary label for grouping input validation, sanitization, encoding, escaping related requirements label Apr 5, 2023
@elarlang
Copy link
Collaborator

elarlang commented Apr 5, 2023

Note: for updated version, move to comment #1552 (comment)

Related requirements 1.5.1, 1.8.1, 5.1.3, 5.1.4.

# Description L1 L2 L3 CWE
1.5.1 Verify that input and output requirements clearly define how to handle and process data based on type, content, and applicable laws, regulations, and other policy compliance. 1029
1.8.1 [MODIFIED, MERGED FROM 8.3.4, LEVEL L2 > L1] Verify that all sensitive data created and processed by the application has been identified and classified into protection levels, and ensure that a policy is in place on how to deal with sensitive data. 213
5.1.3 [MODIFIED] Verify that all input (HTML form fields, REST requests, URL parameters, HTTP headers, cookies, batch files, RSS feeds, etc) is validated using positive validation (allow lists). Where HTML markup must be accepted for input, refer to requirement 5.2.1. (C5) 20
5.1.4 Verify that structured data is strongly typed and validated against a defined schema including allowed characters, length and pattern (e.g. credit card numbers, e-mail addresses, telephone numbers, or validating that two related fields are reasonable, such as checking that suburb and zip/postcode match). (C5) 20

I think 5.1.3 and 5.1.4 are overlapping at the moment and those should be more clear. I think those can be merged (and maybe logically related fields from 5.1.4 as separate requirement)

Goals for requirements:

  • 1.5.1 - everything must be analyzed and documented - otherwise it is not possible to implement input validation and it's not possible to test, is it done correctly.
  • 1.8.1 - everything must be analyzed and documented from "is it sensitive" perspective. It's included here just to point out potential overlap with 1.5.1
  • 5.1.3 everything must be validated using positive validation (allow lists) == do not use disallow/block-lists
  • 5.1.4 every element must be with expected type, contain only allowed characters, in min-max value range for numbers, in min-max length for texts, follow patterns for expected type (phone, email, address, ...), logically related fields are matching

If we have agreement on the requirement goals, then we can start finetune them.

@elarlang elarlang changed the title Adding clarity for 5.1.4 Adding clarity for 5.1.3 and 5.1.4 (related 1.5.1, 1.8.1) Apr 5, 2023
@elarlang elarlang added the next meeting Filter for leaders label Apr 29, 2023
@tghosth tghosth added 4b Major-rework These issues need to be part of a full chapter rework _5.0 - prep This needs to be addressed to prepare 5.0 and removed 1) Discussion ongoing Issue is opened and assigned but no clear proposal yet labels Jul 9, 2023
@elarlang elarlang removed the next meeting Filter for leaders label Nov 29, 2023
@elarlang
Copy link
Collaborator

ping @tghosth @jmanico - do you agree with my definitions (#1552 (comment)) and we can move it further?

@jmanico
Copy link
Member Author

jmanico commented Jan 24, 2024 via email

@elarlang
Copy link
Collaborator

elarlang commented Jan 24, 2024

Yes, I’m with you so far. PS: And I am a bit wary of input validation in general because valid data is often still vulnerable to injection and similar.

Defenses against injection attacks are sanitization, escaping and encoding, it's a separate section.

@tghosth
Copy link
Collaborator

tghosth commented Jan 24, 2024

Agree on your definitions @elarlang

@elarlang elarlang changed the title Adding clarity for 5.1.3 and 5.1.4 (related 1.5.1, 1.8.1) Adding clarity for 1.5.1 and 5.1.4 (related 5.1.3, 1.8.1) Feb 28, 2024
@elarlang
Copy link
Collaborator

This is just an update, to make the focus for the issue more clear.

1.8.1 is solved and for 5.1.3 I opened a separate issue (#1878).

Now we have 1.5.1 and 5.1.4 to solve, currently those are:

# Description L1 L2 L3 CWE
1.5.1 [MODIFIED, LEVEL L2 > L1] Verify that input and output requirements clearly define how to handle and process data based on type and content. 20
5.1.4 [GRAMMAR] Verify that structured data is strongly typed and validated against a defined schema including allowed characters, length and pattern (e.g. credit card numbers, e-mail addresses, telephone numbers, or validating that two related fields are reasonable, such as checking that suburb and zipcode match). (C5) 20

The points listed here are agreed:

  • 1.5.1 - everything must be analyzed and documented - otherwise it is not possible to implement input validation and it's not possible to test, is it done correctly.
  • 5.1.4 - every element must be with expected type, contain only allowed characters, in min-max value range for numbers, in min-max length for texts, follow patterns for expected type (phone, email, address, ...), logically related fields are matching

@jmanico
Copy link
Member Author

jmanico commented Feb 28, 2024

I suggest 1.5.1 goes away and we just keep 5.1.4. 5.1.4 is clear enough.

@elarlang
Copy link
Collaborator

n+1'th time - you can not develop (and later test) without having documentation requirements in place.

@jmanico
Copy link
Member Author

jmanico commented Feb 28, 2024

I agree. But ASVS is already very bulky. Some things are already implied. And no one - ever - got hacked because of missing documentation.

@elarlang
Copy link
Collaborator

If you don't have documentation, on how you need to implement something, there's quite a big chance that you will not implement it (correctly), and... you may get hacked because of that.

But in general, at the moment we create documentation requirements AND implementation requirements. What we will do or how we organize the documentation requirement is up for discussion (in #1831) and out of scope for this issue.

@jmanico
Copy link
Member Author

jmanico commented Jun 6, 2024

I surrender and see the value of 1.5.1. I think we are all in sync now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4b Major-rework These issues need to be part of a full chapter rework Community wanted We would like feedback from the community to guide our decision otherwise we will progress V5 Temporary label for grouping input validation, sanitization, encoding, escaping related requirements _5.0 - prep This needs to be addressed to prepare 5.0
Projects
None yet
Development

No branches or pull requests

6 participants