OWASP / owasp-mastg

The Mobile Application Security Testing Guide (MASTG) is a comprehensive manual for mobile app security testing and reverse engineering. It describes the technical processes for verifying the controls listed in the OWASP Mobile Application Security Verification Standard (MASVS).
https://mas.owasp.org/
Creative Commons Attribution Share Alike 4.0 International
11.57k stars 2.29k forks source link

Overlap between two testcases #52

Closed TmmmmmR closed 7 years ago

TmmmmmR commented 7 years ago

Hello,

I think there is an overlap between the following testcases :

OMTG-CODE-007: Test Input Validation OMTG-CODE-005: Test Exception Handling

I suggest that we keep only OMTG-CODE-005 because OMTG-CODE-007 may lead to other test cases (XSS or SQLi for example).

What do you think ?

sushi2k commented 7 years ago

Hi Abdessamed,

if you look into the MASVS (https://github.com/OWASP/owasp-masvs/blob/master/Document/0x12-V7-Code_quality_and_build_setting_requirements.md#security-verification-requirements) you can see that these requirements address different things.

OMTG-CODE-005 is addressing 7.5 in the MASVS: 7.5 The app catches and handles possible exceptions.

OMTG-CODE-007 is addressing 7.7 in the MASVS: 7.7 No untrusted external input is concatenated into database queries or dynamically executed code.

We need both, as 7.5 is addressing proper error handling and 7.7 is addressing proper input validation in case the input is used within the App (SQL Injection in local SQLite database for example).

Makes sense to you?

TmmmmmR commented 7 years ago

Hello Sven,

I get your point, but my concern is "Exceptions Handling" can be used to Resolve "Input Validation" Issues.

Maybe we should talk about "Improper Exceptions Handling" ?

clviper commented 7 years ago

@TmmmmmR they are different things. Exception Handling does not guarantee that you don't have issues with Input Validation. You can have a vulnerability via user input that does not trigger an Exception. Doesn't make sense to join both.

TmmmmmR commented 7 years ago

@clviper : Yes that's it ! If a "user input that does not trigger an Exception" then the problem is in the "Exception" handling mechanism itself.

I think there is also an overlap between "OMTG-CODE-006: Verify that the App Fails Securely" and "OMTG-CODE-005: Test Exception Handling". Because improper exception handling may lead to a crash.

sushi2k commented 7 years ago

I think you are right, OMTG-CODE-006 and 005 address the same issue. The error/exception handling need to be done properly, if this is done the the App will also fail securely. Any objection to merge these two?

sushi2k commented 7 years ago

@TmmmmmR 007 is a test case on it's own. As @clviper already explained, even if you have proper exception handling, that does not mean that the input validation is according to best practices. If you look into the MASVS, test case 007 is also addressing the following:

"No untrusted external input is concatenated into database queries or dynamically executed code."

See: https://github.com/OWASP/owasp-masvs/blob/master/Document/0x12-V7-Code_quality_and_build_setting_requirements.md#security-verification-requirements

Therefore I would suggest to keep 007.

TmmmmmR commented 7 years ago

@sushi2k : yes we can merge OMTG-CODE-006 and 005 And about OMTG-CODE-007, My concern was that sometimes developer use Exception for input validation. But, since it's a bad practise to use exception to validate input (see : http://stackoverflow.com/questions/7304067/using-exceptions-to-validate-inputs) I agree to keep 007.

sushi2k commented 7 years ago

Thanks for your quick response. Then please merge it also in the MASVS, so MSTG and MASVS are still consistent.

sushi2k commented 7 years ago

This was already addressed by Bernhard in the MASVS, so will close it.