-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Port MASTG test 0044 (by @Guardsquare) (#3049)
* Port MASTG test 0044 * deprecation note * Apply suggestions from code review * Add MASTG-TECH-0115: obtaining compiler provided security features * update IDs * add refs * Update MASTG-TEST-0222: clarify native libraries and deprecate support for non-PIE executables * Update MASTG-TEST-0222: improve title casing and add clarification on PIE requirements * Update MASTG-TEST-0223: improve title casing and enhance clarity on stack canaries and their importance in native libraries * Apply suggestions from code review Co-authored-by: titze <[email protected]> * Update tests-beta/android/MASVS-CODE/MASTG-TEST-0223.md * Update tests-beta/android/MASVS-CODE/MASTG-TEST-0223.md --------- Co-authored-by: Carlos Holguera <[email protected]>
- Loading branch information
1 parent
5c2d9c8
commit 4e8bddf
Showing
5 changed files
with
112 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
--- | ||
title: Obtaining Compiler Provided Security Features | ||
platform: android | ||
--- | ||
|
||
Run @MASTG-TOOL-0028 on the target binary, for example a shared library and grep for the keywords you'd like to check for. | ||
|
||
```sh | ||
rabin2 -I lib/x86_64/libnative-lib.so | grep -E "canary|pic" | ||
canary false | ||
``` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
--- | ||
title: Position Independent Code (PIC) Not Enabled | ||
platform: android | ||
id: MASTG-TEST-0222 | ||
deprecated_since: 21 | ||
type: [static] | ||
weakness: MASWE-0116 | ||
--- | ||
|
||
## Overview | ||
|
||
This test case checks if the [native libraries](../../../Document/0x05i-Testing-Code-Quality-and-Build-Settings/#binary-protection-mechanisms) of the app are compiled without enabling [Position Independent Code (PIC)](../../../Document/0x04h-Testing-Code-Quality/#position-independent-code), a common mitigation technique against memory corruption attacks. | ||
|
||
Since Android 5.0 (API level 21), Android requires [all dynamically linked executables to support PIE](https://source.android.com/docs/security/enhancements/#android-5). | ||
|
||
> [Build System Maintainers Guide - Additional Required Arguments](https://android.googlesource.com/platform/ndk/%2B/master/docs/BuildSystemMaintainers.md#additional-required-arguments): Android requires Position-independent executables beginning with API 21. Clang builds PIE executables by default. If invoking the linker directly or not using Clang, use `-pie` when linking. | ||
## Steps | ||
|
||
1. Extract the app contents (@MASTG-TECH-0007). | ||
2. Run @MASTG-TECH-0115 on each shared library and grep for "pic" or the corresponding keyword used by the selected tool. | ||
|
||
## Observation | ||
|
||
The output should list if PIC is enabled or disabled. | ||
|
||
## Evaluation | ||
|
||
The test case fails if PIC is disabled. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
--- | ||
title: Stack Canaries Not Enabled | ||
platform: android | ||
id: MASTG-TEST-0223 | ||
type: [static] | ||
weakness: MASWE-0116 | ||
--- | ||
|
||
## Overview | ||
|
||
This test case checks if the [native libraries](../../../Document/0x05i-Testing-Code-Quality-and-Build-Settings/#binary-protection-mechanisms) of the app are compiled without stack canaries and therefore lacking [stack smashing protection](../../../Document/0x04h-Testing-Code-Quality/#stack-smashing-protection), a common mitigation technique against buffer overflow attacks. | ||
|
||
- NDK libraries should have stack canaries enabled since [the compiler does it by default](https://android.googlesource.com/platform/ndk/%2B/master/docs/BuildSystemMaintainers.md#additional-required-arguments). | ||
- Other custom C/C++ libraries might not have stack canaries enabled because they lack the necessary compiler flags (`-fstack-protector-strong`, or `-fstack-protector-all`) or the canaries were optimized out by the compiler. See the [Evaluation](#evaluation) section for more details. | ||
|
||
## Steps | ||
|
||
1. Extract the app contents (@MASTG-TECH-0058). | ||
2. Run @MASTG-TECH-0115 on each shared library and grep for "canary" or the corresponding keyword used by the selected tool. | ||
|
||
## Observation | ||
|
||
The output should show if stack canaries are enabled or disabled. | ||
|
||
## Evaluation | ||
|
||
The test case fails if stack canaries are disabled. | ||
|
||
Developers need to ensure that the flags `-fstack-protector-strong`, or `-fstack-protector-all` are set in the compiler flags for all native libraries. This is especially important for custom C/C++ libraries that are not part of the NDK. | ||
|
||
When evaluating this please note that there are potential **expected false positives** for which the test case should be considered as passed. To be certain for these cases, they require manual review of the original source code and the compilation flags used. | ||
|
||
The following examples cover some of the false positive cases that might be encountered: | ||
|
||
### Use of Memory Safe Languages | ||
|
||
The Flutter framework does not use stack canaries because of the way [Dart mitigates buffer overflows](https://docs.flutter.dev/reference/security-false-positives#shared-objects-should-use-stack-canary-values). | ||
|
||
### Compiler Optimizations | ||
|
||
Sometimes, due to the size of the library and the optimizations applied by the compiler, it might be possible that the library was originally compiled with stack canaries but they were optimized out. For example, this is the case for some [react native apps](https://github.com/facebook/react-native/issues/36870#issuecomment-1714007068). They are built with `-fstack-protector-strong` but when attempting to search for `stack_chk_fail` inside the `.so` files, it is not found. | ||
|
||
- **Empty .so files**: Some .so files such as `libruntimeexecutor.so` or `libreact_render_debug.so` are effectively empty in release and therefore contain no symbols. Even if you were to attempt to build with `-fstack-protector-all`, you still won't be able to see the `stack_chk_fail` string as there are no method calls there. | ||
- **Lack of stack buffer calls**: Other files such as `libreact_utils.so`, `libreact_config.so`, and `libreact_debug.so` are not empty and contain method calls, but those methods don't contain stack buffer calls, so there are no `stack_chk_fail` strings inside them. | ||
|
||
The React Native developers in this case declare that they won't be adding `-fstack-protector-all` as, in their case, [they consider that doing so will add a performance hit for no effective security gain](https://github.com/OWASP/owasp-mastg/pull/3049#pullrequestreview-2420837259). |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
--- | ||
title: Compiler Provided Security Features Not Used | ||
id: MASWE-0116 | ||
alias: compiler-provided-security-features-not-implemented | ||
platform: [android, ios] | ||
profiles: [L2] | ||
mappings: | ||
masvs-v2: [MASVS-CODE-3, MASVS-CODE-4] | ||
refs: | ||
- https://cs.android.com/android/platform/superproject/main/+/main:bionic/linker/linker_main.cpp;l=397?q=linker_main&ss=android%2Fplatform%2Fsuperproject%2Fmain | ||
- https://www.mcafee.com/enterprise/en-us/assets/white-papers/wp-secure-coding-android-applications.pdf | ||
- https://mas.owasp.org/MASTG/0x05i-Testing-Code-Quality-and-Build-Settings/#binary-protection-mechanisms | ||
- https://mas.owasp.org/MASTG/0x06i-Testing-Code-Quality-and-Build-Settings/#binary-protection-mechanisms | ||
draft: | ||
description: e.g., PIC, stack canaries. Alternative title could be Memory Anti-Exploitation Mechanisms Not Implemented | ||
topics: | ||
- PIC | ||
- stack canaries | ||
note: PIC cannot be switched off in newer versions of Android, the NDK does not link against such libraries anymore [source](https://cs.android.com/android/platform/superproject/main/+/main:bionic/linker/linker_main.cpp;l=397?q=linker_main&ss=android%2Fplatform%2Fsuperproject%2Fmain). | ||
status: draft | ||
|
||
--- | ||
|