Skip to content

Conversation

jeel38
Copy link
Collaborator

@jeel38 jeel38 commented Jun 10, 2025

Closes #2995

This PR ports MASTG-TEST-0002 from version 1 to version 2, creating a new test for detecting use of local storage for input validation in Android applications. The focus shifts from general local storage testing to specifically checking for integrity validation when reading from SharedPreferences.

  • Deprecates the old MASTG-TEST-0002 and creates MASTG-TEST-0288 as the new version
  • Implements static analysis detection for SharedPreferences usage without integrity checks
  • Provides demonstration code showing both vulnerable and secure patterns using HMAC validation
File Description
tests/android/MASVS-CODE/MASTG-TEST-0002.md Marks the original test as deprecated with reference to new version
tests-beta/android/MASVS-CODE/MASTG-TEST-0288.md New test specification for detecting SharedPreferences without integrity validation
rules/mastg-android-local-storage-input-validation.yml Semgrep rule to detect insecure SharedPreferences usage patterns
demos/android/MASVS-CODE/MASTG-DEMO-0061/ Complete demonstration including vulnerable code, and analysis output

@jeel38
Copy link
Collaborator Author

jeel38 commented Jun 10, 2025

@cpholguera, upon adding the depreciation note, getting branch gets a conflict

@cpholguera
Copy link
Collaborator

Maybe the branch was a bit old. We added "profiles" now for all tests. I fixed the conflict for you

Please add profiles to tests-beta/android/MASVS-CODE/MASTG-TEST-0281.md as well

@cpholguera
Copy link
Collaborator

The new profiles (previously known as "levels") are defined here: https://docs.google.com/document/d/1paz7dxKXHzAC9MN7Mnln1JiZwBNyg7Gs364AJ6KudEs/edit?usp=sharing

@cpholguera cpholguera requested a review from Copilot June 10, 2025 08:36
Copilot

This comment was marked as outdated.

@jeel38
Copy link
Collaborator Author

jeel38 commented Jun 10, 2025

Maybe the branch was a bit old. We added "profiles" now for all tests. I fixed the conflict for you

Please add profiles to tests-beta/android/MASVS-CODE/MASTG-TEST-0281.md as well

Done

@jeel38
Copy link
Collaborator Author

jeel38 commented Jun 10, 2025

@cpholguera please review it

@cpholguera
Copy link
Collaborator

Please fix the issues reported by Copilot in section "Comments suppressed due to low confidence (5)"


Also, I agree with Copilot's comment about:

"The output file shows usages of the input validation using putString and getString in the code."

The phrasing is misleading: this test detects storage/retrieval calls, not input validation.

If you're conducting a pentest, would you report this as an issue? "The app uses putString() and getString()." The same goes for an automated security testing tool, would you like it to report this? Wouldn't that be noisy / lots of false positives?

@cpholguera
Copy link
Collaborator

There's another problem that will probably be solved when you fix the copilot issues. Are you able to read these logs?

https://github.com/OWASP/owasp-mastg/actions/runs/15554645412/job/43792346017?pr=3328

ERROR - Error reading page 'MASTG/demos/android/MASVS-CODE/MASTG-DEMO-0054/MASTG-DEMO-0054.md': Snippet at path '../../../../rules/mastg-android-local-storage-for-input-validation.yml' could not be found

@jeel38
Copy link
Collaborator Author

jeel38 commented Aug 11, 2025

@cpholguera please check

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR ports MASTG-TEST-0002 from version 1 to version 2, creating a new test for detecting use of local storage for input validation in Android applications. The focus shifts from general local storage testing to specifically checking for integrity validation when reading from SharedPreferences.

  • Deprecates the old MASTG-TEST-0002 and creates MASTG-TEST-0288 as the new version
  • Implements static analysis detection for SharedPreferences usage without integrity checks
  • Provides demonstration code showing both vulnerable and secure patterns using HMAC validation

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

File Description
tests/android/MASVS-CODE/MASTG-TEST-0002.md Marks the original test as deprecated with reference to new version
tests-beta/android/MASVS-CODE/MASTG-TEST-0288.md New test specification for detecting SharedPreferences without integrity validation
rules/mastg-android-local-storage-input-validation.yml Semgrep rule to detect insecure SharedPreferences usage patterns
demos/android/MASVS-CODE/MASTG-DEMO-0061/ Complete demonstration including vulnerable Java code, secure Kotlin implementation, and analysis output

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@@ -9,6 +9,9 @@ masvs_v1_levels:
- L1
- L2
profiles: [L1, L2]
status: deprecated
covered_by: [MASTG-TEST-0281]
Copy link
Preview

Copilot AI Aug 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The deprecation references MASTG-TEST-0281, but the actual replacement test created in this PR is MASTG-TEST-0288. This should be corrected to reference the correct test ID.

Suggested change
covered_by: [MASTG-TEST-0281]
covered_by: [MASTG-TEST-0288]

Copilot uses AI. Check for mistakes.

Comment on lines +10 to +11
- pattern: new SecureSharedPreferences($CONTEXT, false)

Copy link
Preview

Copilot AI Aug 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pattern only detects SecureSharedPreferences with false parameter, but the rule description mentions detecting SharedPreferences usage without integrity checks. This pattern is too narrow and won't catch standard SharedPreferences.getString() calls or other SharedPreferences access patterns that lack integrity validation.

Suggested change
- pattern: new SecureSharedPreferences($CONTEXT, false)
- pattern: $SP.getString($KEY, $DEFAULT)
- pattern: $SP.getInt($KEY, $DEFAULT)
- pattern: $CONTEXT.getSharedPreferences($NAME, $MODE)

Copilot uses AI. Check for mistakes.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please test properly before adding this Copilot suggestion to validate if it makes sense and works.


## Steps

1. Run a static analysis tool such as @MASTG-TOOL-0110 on the code and look for uses of the `putString` and `getString`.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an old file

Comment on lines +10 to +11
- pattern: new SecureSharedPreferences($CONTEXT, false)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please test properly before adding this Copilot suggestion to validate if it makes sense and works.


### Evaluation

The test fails as the code does not use an `HMAC` integrity check together with `SharedPreferences` data.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please describe how, because of this, an attacker can exploit the app. Use references to techniques when possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MASTG v1->v2 MASTG-TEST-0002: Testing Local Storage for Input Validation (android)
2 participants