summaryrefslogtreecommitdiff
path: root/cmds/screencap
diff options
context:
space:
mode:
authorDichen Zhang <dichenzhang@google.com>2020-03-12 12:25:09 -0700
committerDichen Zhang <dichenzhang@google.com>2020-03-26 00:09:21 +0000
commiteab2d8c0463dc29e594d6422a5610b729adce14b (patch)
tree05fc5029f97362668375819302137edddecb3a60 /cmds/screencap
parent56e3510bda83985780e89b60c5ebcce458731933 (diff)
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. Cherry-pick from ag/10651695 with fix ag/10700515 Bug: 123230379 Test: please see commands #4 and #5 Change-Id: Icd88cdf4af153e07addb4449cdb117b1a3c881d3
Diffstat (limited to 'cmds/screencap')
-rw-r--r--cmds/screencap/screencap.cpp38
1 files changed, 33 insertions, 5 deletions
diff --git a/cmds/screencap/screencap.cpp b/cmds/screencap/screencap.cpp
index 0bb1af13643c..4410f1c4570c 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,38 @@ 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 *cmd[] = {
+ (char*) "am",
+ (char*) "broadcast",
+ (char*) "am",
+ (char*) "android.intent.action.MEDIA_SCANNER_SCAN_FILE",
+ (char*) "-d",
+ &filePath[0],
+ 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;
}