Skip to content

Commit 5988ae2

Browse files
pengdevgithub-actions[bot]
authored andcommitted
Fix command injection vulnerabilities in maps-android Python scripts (#10424)
### Summary Fixes https://mapbox.atlassian.net/browse/MAPSAND-2556 - Deleted dead CI script with shell injection via `--config` input - Fixed active CI script `check-permissions.py` to use safe subprocess list form ### Problem Two Python scripts in `projects/maps-android/mapbox-maps-android/scripts/` contained command injection vulnerabilities via `shell=True` with unsanitized user input interpolated directly into shell strings. ### Solution Remove the dead script entirely; patch the active script by replacing the string-form `subprocess.Popen` call with the list form, which bypasses shell interpretation and prevents injection. ### Key Changes - **Deleted** `scripts/ci-e2e-compatibility-start-pipeline.py` — unreferenced in any Android CI config; `execute_command()` passed user-controlled `--config` values into an awk shell string via `shell=True` - **Fixed** `scripts/check-permissions.py` — replaced `subprocess.Popen(f"aapt d permissions {args.apk}", shell=True)` with `subprocess.Popen(["aapt", "d", "permissions", args.apk])` to prevent injection via `--apk` argument ### Note: same vulnerability exists in other projects The deleted script has copies in other projects that carry the same vulnerability but are **out of scope** for this MAPSAND ticket: | Project | File | Referenced in CI? | |---------|------|-------------------| | maps-ios | `scripts/ci-trigger/ci-e2e-compatibility-start-pipeline.py` | Yes — `.circleci/config/jobs/maps-ios-sdk-e2e-test.yml` | | gl-native | (via maps-ios copy) | Yes — `.circleci/generated_config.yml` | | common | `scripts/ci/ci-e2e-compatibility-start-pipeline.py` | Not confirmed | These should be addressed in separate tickets for their respective projects. cc @mapbox/maps-ios @mapbox/gl @mapbox/core-sdk ### Validation - [x] Confirmed `ci-e2e-compatibility-start-pipeline.py` is not referenced in any Android CI config - [x] Grepped the full monorepo — script is only actively used in maps-ios and gl-native CI - [x] `check-permissions.py` behavior is unchanged for valid inputs — only the subprocess invocation form changed - [x] Verify `check-permissions.py` still passes in CI with the patched invocation cc @mapbox/maps-android GitOrigin-RevId: 6551a2b1e470adc4cc3098897fe8c3e93c0a226d
1 parent ef32eb9 commit 5988ae2

File tree

2 files changed

+1
-167
lines changed

2 files changed

+1
-167
lines changed

scripts/check-permissions.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99

1010
def load_apk_permissions(args):
1111
permissions = []
12-
shell = subprocess.Popen(f"aapt d permissions {args.apk}", stdout=subprocess.PIPE, shell=True).stdout
12+
shell = subprocess.Popen(["aapt", "d", "permissions", args.apk], stdout=subprocess.PIPE).stdout
1313
for output in shell:
1414
line = output.decode('utf-8')
1515
if USES_PERMISSION in line:

scripts/ci-e2e-compatibility-start-pipeline.py

Lines changed: 0 additions & 166 deletions
This file was deleted.

0 commit comments

Comments
 (0)