From 4e8bddfde911fe6205b665494fde11b238018318 Mon Sep 17 00:00:00 2001 From: titze Date: Thu, 7 Nov 2024 15:18:24 +0100 Subject: [PATCH] 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 * 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 --- techniques/android/MASTG-TECH-0115.md | 11 +++++ .../android/MASVS-CODE/MASTG-TEST-0222.md | 29 ++++++++++++ .../android/MASVS-CODE/MASTG-TEST-0223.md | 46 +++++++++++++++++++ tests/android/MASVS-CODE/MASTG-TEST-0044.md | 3 ++ weaknesses/MASVS-CODE/MASWE-0116.md | 23 ++++++++++ 5 files changed, 112 insertions(+) create mode 100644 techniques/android/MASTG-TECH-0115.md create mode 100644 tests-beta/android/MASVS-CODE/MASTG-TEST-0222.md create mode 100644 tests-beta/android/MASVS-CODE/MASTG-TEST-0223.md create mode 100644 weaknesses/MASVS-CODE/MASWE-0116.md diff --git a/techniques/android/MASTG-TECH-0115.md b/techniques/android/MASTG-TECH-0115.md new file mode 100644 index 0000000000..0864e845d8 --- /dev/null +++ b/techniques/android/MASTG-TECH-0115.md @@ -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 +``` diff --git a/tests-beta/android/MASVS-CODE/MASTG-TEST-0222.md b/tests-beta/android/MASVS-CODE/MASTG-TEST-0222.md new file mode 100644 index 0000000000..d93dad8af3 --- /dev/null +++ b/tests-beta/android/MASVS-CODE/MASTG-TEST-0222.md @@ -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. diff --git a/tests-beta/android/MASVS-CODE/MASTG-TEST-0223.md b/tests-beta/android/MASVS-CODE/MASTG-TEST-0223.md new file mode 100644 index 0000000000..aeb7310926 --- /dev/null +++ b/tests-beta/android/MASVS-CODE/MASTG-TEST-0223.md @@ -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). diff --git a/tests/android/MASVS-CODE/MASTG-TEST-0044.md b/tests/android/MASVS-CODE/MASTG-TEST-0044.md index d0be67ba87..b8a8769813 100644 --- a/tests/android/MASVS-CODE/MASTG-TEST-0044.md +++ b/tests/android/MASVS-CODE/MASTG-TEST-0044.md @@ -8,6 +8,9 @@ title: Make Sure That Free Security Features Are Activated masvs_v1_levels: - L1 - L2 +status: deprecated +covered_by: [MASTG-TEST-0222, MASTG-TEST-0223] +deprecation_note: New version available in MASTG V2 --- ## Overview diff --git a/weaknesses/MASVS-CODE/MASWE-0116.md b/weaknesses/MASVS-CODE/MASWE-0116.md new file mode 100644 index 0000000000..9ef191053f --- /dev/null +++ b/weaknesses/MASVS-CODE/MASWE-0116.md @@ -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 + +--- +