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

V11 rework by @jmanico #1953

Open
tghosth opened this issue May 7, 2024 · 15 comments
Open

V11 rework by @jmanico #1953

tghosth opened this issue May 7, 2024 · 15 comments
Assignees
Labels
4b Major-rework These issues need to be part of a full chapter rework V11 _5.0 - prep This needs to be addressed to prepare 5.0

Comments

@tghosth
Copy link
Collaborator

tghosth commented May 7, 2024

Jim has prepared a rework of V11:
https://github.com/OWASP/ASVS/tree/Business-Logic-5.0-Rewrite

These are the notes that Jim sent:

  1. Drop 11.1.5 - its too general
  2. Move 11.1.7 and 11.1.8 into a new section "11.3 Monitoring"
  3. Drop references to AppSensor and Cornucopia
  4. Re-write the definition per Requesting Clarifying Definition in the Business Logic Section Header #1869

This shows V11 status:
GitHub issue custom search

@tghosth tghosth added _5.0 - prep This needs to be addressed to prepare 5.0 4b Major-rework These issues need to be part of a full chapter rework V11 labels May 7, 2024
@tghosth
Copy link
Collaborator Author

tghosth commented May 7, 2024

@jmanico did you have an opinion on:

@elarlang
Copy link
Collaborator

elarlang commented May 7, 2024

We need this requirement. If you don't have the limits defined, you can not implement them and you can not test them. It's a documentation and pre-condition requirement that, by current logic belongs to section V1.11.

# Description L1 L2 L3 CWE
11.1.5 [MODIFIED] Verify that the application has globally defined business logic limits or validation to protect against likely business risks or threats, identified using threat modeling or similar methodologies.

@tghosth
Copy link
Collaborator Author

tghosth commented May 7, 2024

We need this requirement. If you don't have the limits defined, you can not implement them and you can not test them. It's a documentation and pre-condition requirement that, by current logic belongs to section V1.11.

# Description L1 L2 L3 CWE
11.1.5 [MODIFIED] Verify that the application has globally defined business logic limits or validation to protect against likely business risks or threats, identified using threat modeling or similar methodologies.

Can you think how we can reword it to be more specific or clear that this is what we are trying to achieve?

@elarlang
Copy link
Collaborator

elarlang commented May 7, 2024

"Monitoring": I have addressed the topic here: #1211 (comment)

And those requirement are moved to V7 in another branch:
b1545c2

@elarlang
Copy link
Collaborator

elarlang commented May 7, 2024

Can you think how we can reword it to be more specific or clear that this is what we are trying to achieve?

Using the same style as other similar ones:

# 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
1.7.3 [ADDED] Verify that an inventory exists documenting the logging performed at each layer of the application's technology stack, what events are being logged, log formats, where that logging is stored, how it is used, how access to it is controlled and how long logs are kept for. 778
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

@jmanico
Copy link
Member

jmanico commented May 7, 2024

11.1.5 "Globally defined" means that software needs to track global totals that typically is beyond the current software. All the more reason why we need to support dual-authorization.

@elarlang
Copy link
Collaborator

elarlang commented May 7, 2024

The "Globally" is unnecessary and limiting here.

All I say, we just need a requirement to say (not a wording proposal!) "Verify that business logic limits are analyzed and documented." + "Verify that business logic limits are implemented correctly".

@jmanico
Copy link
Member

jmanico commented May 7, 2024

We already have per-user limits defined. I think this is clear enough so it's easily implemented.

11.1.3 [MODIFIED] Verify that the application has appropriate limits defined on a per user basis for specific business actions or transactions. ✓ ✓ ✓

The 4x requirement that defines multi-user approval will cover the global limits. We can delete it from section v11.

@tghosth
Copy link
Collaborator Author

tghosth commented May 9, 2024

@jmanico I discussed with @elarlang and we thought this made sense and is in line with our current thinking about documentation requirements

# Description L1 L2 L3 CWE
1.11.4 [ADDED] Verify that expectations for business logic limits and validations are clearly documented including both per-user and also globally across the application.
11.1.3 [MODIFIED, MERGED FROM 11.1.5] Verify that business logic limits and validations implemented as per the application's documentation.
11.1.5 [DELETED, MERGED TO 11.1.3]

@jmanico
Copy link
Member

jmanico commented May 9, 2024 via email

jmanico added a commit that referenced this issue May 9, 2024
@jmanico
Copy link
Member

jmanico commented May 9, 2024

PR submitted via #1954 that should address everyone's comments. I also updated the definition and removed some of the older references.

elarlang added a commit to elarlang/ASVS that referenced this issue May 9, 2024
* Resolve OWASP#1863 by clarifying 13.4.2

* Remove blanks

* tag

---------

Co-authored-by: Elar Lang <47597707+elarlang@users.noreply.github.com>
@tghosth
Copy link
Collaborator Author

tghosth commented May 16, 2024

@elarlang do you think we need anything else on #1954?

@elarlang
Copy link
Collaborator

I have addressed those here: #1869 (comment)

@tghosth
Copy link
Collaborator Author

tghosth commented May 19, 2024

I don't think that issue is a blocker for merging but should certainly be addressed when we have a draft for 5.0

@tghosth
Copy link
Collaborator Author

tghosth commented May 19, 2024

Now just waiting on #1576

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 V11 _5.0 - prep This needs to be addressed to prepare 5.0
Projects
None yet
Development

No branches or pull requests

3 participants