diff options
author | Dichen Zhang <dichenzhang@google.com> | 2020-03-12 12:25:09 -0700 |
---|---|---|
committer | Dichen Zhang <dichenzhang@google.com> | 2020-03-26 00:09:21 +0000 |
commit | eab2d8c0463dc29e594d6422a5610b729adce14b (patch) | |
tree | 05fc5029f97362668375819302137edddecb3a60 /cmds/screencap/screencap.cpp | |
parent | 56e3510bda83985780e89b60c5ebcce458731933 (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/screencap.cpp')
-rw-r--r-- | cmds/screencap/screencap.cpp | 38 |
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; } |