From d31f219f2d458bd323ae9585a5f2d87e3f4b07fd Mon Sep 17 00:00:00 2001
From: Dichen Zhang <dichenzhang@google.com>
Date: Thu, 12 Mar 2020 12:25:09 -0700
Subject: [PATCH] Fix command injection on screencap

There is a potential injection by using screencap in case of user handled parameters.
"dumpstate" command launches "screencap", when "-p" is argument is set. At that moment, content of "-o" parameter generates a path with ".png" extension to define "screencap" argument.
"dumpstate" is often run as a service with "root" privileged such as defined in "dumpstate.rc". For instance "bugreportz" call "ctl.start" property with "dumpstatez".
Launching "dumpstate" with "-p" option and a user input as "-o" would result in a root command execution. SE Linux might protect part of this attack.

Bug: 123230379
Test: please see commands #4 and #5
Change-Id: Icd88cdf4af153e07addb4449cdb117b1a3c881d3
---
 cmds/screencap/screencap.cpp | 39 +++++++++++++++++++++++++++++++-----
 1 file changed, 34 insertions(+), 5 deletions(-)

diff --git a/cmds/screencap/screencap.cpp b/cmds/screencap/screencap.cpp
index 0bb1af13643c8..522c272f42552 100644
--- a/cmds/screencap/screencap.cpp
+++ b/cmds/screencap/screencap.cpp
@@ -24,6 +24,7 @@
 #include <linux/fb.h>
 #include <sys/ioctl.h>
 #include <sys/mman.h>
+#include <sys/wait.h>
 
 #include <binder/ProcessState.h>
 
@@ -99,11 +100,39 @@ static uint32_t dataSpaceToInt(ui::Dataspace d)
 }
 
 static status_t notifyMediaScanner(const char* fileName) {
-    String8 cmd("am broadcast -a android.intent.action.MEDIA_SCANNER_SCAN_FILE -d file://");
-    cmd.append(fileName);
-    cmd.append(" > /dev/null");
-    int result = system(cmd.string());
-    if (result < 0) {
+    std::string filePath("file://");
+    filePath.append(fileName);
+    char* fp_arg = strdup(filePath.c_str());
+    char *cmd[] = {
+        (char*) "am",
+        (char*) "broadcast",
+        (char*) "am",
+        (char*) "android.intent.action.MEDIA_SCANNER_SCAN_FILE",
+        (char*) "-d",
+        (char*) fp_arg,
+        nullptr
+    };
+
+    int status;
+    int pid = fork();
+    if (pid < 0){
+       fprintf(stderr, "Unable to fork in order to send intent for media scanner.\n");
+       return UNKNOWN_ERROR;
+    }
+    if (pid == 0){
+        int fd = open("/dev/null", O_WRONLY);
+        if (fd < 0){
+            fprintf(stderr, "Unable to open /dev/null for media scanner stdout redirection.\n");
+            exit(1);
+        }
+        dup2(fd, 1);
+        int result = execvp(cmd[0], cmd);
+        close(fd);
+        exit(result);
+    }
+    wait(&status);
+
+    if (status < 0) {
         fprintf(stderr, "Unable to broadcast intent for media scanner.\n");
         return UNKNOWN_ERROR;
     }