ASVS
ASVS copied to clipboard
OWASP
7.4.2 seems so obvious it's almost insulting to developers
Developers know exception handling well. Please consider clarifying 7.4.2 so it's more security-specific and clear. We already have to use exceptions to do our job.
From: https://github.com/OWASP/ASVS/blob/master/5.0/en/0x15-V7-Error-Logging.md
| 7.4.2 | Verify that exception handling (or a functional equivalent) is used across the codebase to account for expected and unexpected error conditions. (C10) | ✓ | ✓ | 544 |
|---|
Not a nice example how to use wording in issue :)
A lot of program codes do not use exception handling and this requirement is for handling that. If exception handling is in place, then stay calm and keep coding.
Not a nice example how to use wording in issue :)
A lot of program codes do not use exception handling and this requirement is for handling that. If exception handling is in place, then stay calm and keep coding.
I agree, I changed the language to be a bit more professional.
@jmanico - can you clarify, what exactly is wrong with this requirement?
For me the requirement is ok as it is. Applications must have exception handling in place. If it is too obvious for someone, it does not mean it is the same obvious for everyone. If encoding for me is too obvious I don't remove related requirement because of that.
Exception handling is so basic this requirement is not helping much at all. And exception handling mistakes do not necessarily cause security problems. So no beuno, I still think we need to drop this.
I exception is not handled, it can not provide default/intended output and logging from the situation V7.4.1 Verify that a generic message is shown when an unexpected or security sensitive error occurs, potentially with a unique ID which support personnel can use to investigate.
The argument "so basic" is personal point of view. I copy my previous comment, as it is still valid: If it is too obvious for someone, it does not mean it is the same obvious for everyone. If encoding for me is too obvious I don't remove related requirement because of that.
I just closed issue with discussion that "password strength meter must stay" which is more "user education than security". If I compare those 2 - correct exception handling is way more important thing to have in the application. Even it is "too obvious" for someone. The requirement is for those, for whom it is not too obvious or happened to make mistake. And the last one may happen also with those, for whom it's too obvious.
If there is no text improvement proposals, I prefer we keep it as is and close this issue.
I agree with @elarlang. It's like saying that buffer overflows are so basic that they should be excluded. Yes, they're basic and everyone should know about them. But - as they keep turning up every other day - it's clear that many developers don't know about them.
We going to keep the requirement.
Please consider clarifying 7.4.2 so it's more security-specific and clear.
@jmanico - do you have any proposals how to improve it or the wording itself is ok and clear?
@jmanico my dear friend, do you know what item C10 of the OWASP Proactive Controls 2018 is? 😉
Seriously though, I think we should leave this for now but I am open to suggestions for more specific wording 🙂
I still suggest we drop 7.4.2 because it's well covered in 7.4.3 or merge the two.
Current requirements:
| # | Description | L1 | L2 | L3 | CWE |
|---|---|---|---|---|---|
| 7.4.2 | Verify that exception handling (or a functional equivalent) is used across the codebase to account for expected and unexpected error conditions. (C10) | ✓ | ✓ | 544 | |
| 7.4.3 | Verify that a "last resort" error handler is defined which will catch all unhandled exceptions. (C10) | ✓ | ✓ | 431 |
Exception handling as such must have clear requirement.
I can agree that those can be merged - If 7.4.2 is ok, we can not complain about 7.4.3 and vice versa.
Suggestion:
| 7.4.3 | Verify that exception handling (or a functional equivalent) is used across the codebase to account for expected and unexpected error conditions and a "last resort" error handler is defined which will catch all unhandled exceptions. (C10) | ✓ | ✓ | 431 & 544 |
|---|
7.4.3 Verify that a "last resort" error handler is defined which will catch all unhandled exceptions. (C10) ✓ ✓ 431
Firstly, apologies to @jmanico, I closed this issue whereas I should have kept it open until a consensus had been reached.
Looking at the CWEs, I am not convinced that merging these two issues is the right approach. The CWEs describe two different issues
I would suggest keeping both requirements but rewording as follows to be more specific and less obvious:
| # | Description | L1 | L2 | L3 | CWE |
|---|---|---|---|---|---|
| 7.4.2 | Verify that a consistent and standardized exception handling mechanism (or a functional equivalent) is used across the codebase ~~to account for expected and unexpected error conditions~~. (C10) | ✓ | ✓ | 544 | |
| 7.4.3 | Verify that a "last resort" error handler is defined which will catch all unhandled exceptions. (C10) | ✓ | ✓ | 431 |
I think the point of CWE 544 is that a consistent mechanism is being used. Developers know how to exception handling but we want to make sure it is consistent.
If 7.4.2 is ok, we can not complain about 7.4.3 and vice versa.
@tghosth - what do you think about that?
I am ok if you are ready to delete this. I see so many exceptions that do not cause security problems hence my concern but am ok to drop this.
If 7.4.2 is ok, we can not complain about 7.4.3 and vice versa.
@tghosth - what do you think about that?
Hi @elarlang, I think my changes make it so that they no longer contradict each other. They can have a last resort error handler but not a consistent approach to error handling. Similarly, they can have consistent error handling but not have configured a "last resort" handler
As such, I stand by my suggestion above:
# Description L1 L2 L3 CWE 7.4.2 Verify that a consistent and standardized exception handling mechanism (or a functional equivalent) is used across the codebase ~~to account for expected and unexpected error conditions~~. (C10) ✓ ✓ 544 7.4.3 Verify that a "last resort" error handler is defined which will catch all unhandled exceptions. (C10) ✓ ✓ 431
I created a PR #1647 but I will ask @jmanico to review so please don't merge unless he approves