From dcd2f0ef2f87a3fcade74aec7e178be14d0ea335 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Tue, 10 Jun 2014 11:31:24 -0700 Subject: [PATCH 1/2] Uniformly escape shell arguments. Arguments with embedded spaces need to be wrapped in quotes, which changes the overall escaping strategy. Instead of mixing the two strategies, just always wrap arguments in quotes. (cherry-pick of fd546e8c35341b518873eb4f883afbed92e947af.) Bug: 15479704 Change-Id: I03eacfa1bd6c220d4ec6617b825ebb0c43c7221e --- adb/commandline.c | 65 ++++++++++++++++++----------------------------- 1 file changed, 25 insertions(+), 40 deletions(-) diff --git a/adb/commandline.c b/adb/commandline.c index 1ba60499c..8189a710a 100644 --- a/adb/commandline.c +++ b/adb/commandline.c @@ -544,39 +544,33 @@ static void status_window(transport_type ttype, const char* serial) } } -/** duplicate string and quote all \ " ( ) chars + space character. */ -static char * -dupAndQuote(const char *s) +/** Duplicate and escape given argument. */ +static char *escape_argv(const char *s) { const char *ts; size_t alloc_len; char *ret; char *dest; - ts = s; - - alloc_len = 0; - - for( ;*ts != '\0'; ts++) { + alloc_len = 2; + for (ts = s; *ts != '\0'; ts++) { alloc_len++; - if (*ts == ' ' || *ts == '"' || *ts == '\\' || *ts == '(' || *ts == ')') { + if (*ts == '"' || *ts == '\\') { alloc_len++; } } - ret = (char *)malloc(alloc_len + 1); - - ts = s; + ret = (char *) malloc(alloc_len + 1); dest = ret; - for ( ;*ts != '\0'; ts++) { - if (*ts == ' ' || *ts == '"' || *ts == '\\' || *ts == '(' || *ts == ')') { + *dest++ = '"'; + for (ts = s; *ts != '\0'; ts++) { + if (*ts == '"' || *ts == '\\') { *dest++ = '\\'; } - *dest++ = *ts; } - + *dest++ = '"'; *dest++ = '\0'; return ret; @@ -683,30 +677,24 @@ static int logcat(transport_type transport, char* serial, int argc, char **argv) char buf[4096]; char *log_tags; - char *quoted_log_tags; + char *quoted; log_tags = getenv("ANDROID_LOG_TAGS"); - quoted_log_tags = dupAndQuote(log_tags == NULL ? "" : log_tags); - + quoted = escape_argv(log_tags == NULL ? "" : log_tags); snprintf(buf, sizeof(buf), - "shell:export ANDROID_LOG_TAGS=\"\%s\" ; exec logcat", - quoted_log_tags); + "shell:export ANDROID_LOG_TAGS=%s ; exec logcat", quoted); + free(quoted); - free(quoted_log_tags); - - if (!strcmp(argv[0],"longcat")) { - strncat(buf, " -v long", sizeof(buf)-1); + if (!strcmp(argv[0], "longcat")) { + strncat(buf, " -v long", sizeof(buf) - 1); } argc -= 1; argv += 1; while(argc-- > 0) { - char *quoted; - - quoted = dupAndQuote (*argv++); - - strncat(buf, " ", sizeof(buf)-1); - strncat(buf, quoted, sizeof(buf)-1); + quoted = escape_argv(*argv++); + strncat(buf, " ", sizeof(buf) - 1); + strncat(buf, quoted, sizeof(buf) - 1); free(quoted); } @@ -1218,7 +1206,7 @@ top: argc -= 2; argv += 2; while (argc-- > 0) { - char *quoted = dupAndQuote(*argv++); + char *quoted = escape_argv(*argv++); strncat(buf, " ", sizeof(buf) - 1); strncat(buf, quoted, sizeof(buf) - 1); free(quoted); @@ -1261,7 +1249,7 @@ top: argc -= 2; argv += 2; while (argc-- > 0) { - char *quoted = dupAndQuote(*argv++); + char *quoted = escape_argv(*argv++); strncat(buf, " ", sizeof(buf) - 1); strncat(buf, quoted, sizeof(buf) - 1); free(quoted); @@ -1686,12 +1674,9 @@ static int pm_command(transport_type transport, char* serial, snprintf(buf, sizeof(buf), "shell:pm"); while(argc-- > 0) { - char *quoted; - - quoted = dupAndQuote(*argv++); - - strncat(buf, " ", sizeof(buf)-1); - strncat(buf, quoted, sizeof(buf)-1); + char *quoted = escape_argv(*argv++); + strncat(buf, " ", sizeof(buf) - 1); + strncat(buf, quoted, sizeof(buf) - 1); free(quoted); } @@ -1723,7 +1708,7 @@ static int delete_file(transport_type transport, char* serial, char* filename) char* quoted; snprintf(buf, sizeof(buf), "shell:rm "); - quoted = dupAndQuote(filename); + quoted = escape_argv(filename); strncat(buf, quoted, sizeof(buf)-1); free(quoted); From 0cc642c82d94affd5804b7c28c2f2578b8fadafb Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Tue, 10 Jun 2014 16:22:17 -0700 Subject: [PATCH 2/2] Partially revert argument escaping. Commands chained with && need to be passed through literally instead of always being quoted. (cherry-pick of 7c460351f53cb683097fe4071b9ec1e4cd7cdf82.) Bug: 15479704 Change-Id: I2998e40a92a3bfd092098cd526403b469c86c9a6 --- adb/commandline.c | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/adb/commandline.c b/adb/commandline.c index 8189a710a..ccf158c44 100644 --- a/adb/commandline.c +++ b/adb/commandline.c @@ -545,32 +545,39 @@ static void status_window(transport_type ttype, const char* serial) } /** Duplicate and escape given argument. */ -static char *escape_argv(const char *s) +static char *escape_arg(const char *s) { const char *ts; size_t alloc_len; char *ret; char *dest; - alloc_len = 2; + alloc_len = 0; for (ts = s; *ts != '\0'; ts++) { alloc_len++; - if (*ts == '"' || *ts == '\\') { + if (*ts == ' ' || *ts == '"' || *ts == '\\' || *ts == '(' || *ts == ')') { alloc_len++; } } + if (alloc_len == 0) { + // Preserve empty arguments + ret = (char *) malloc(3); + ret[0] = '\"'; + ret[1] = '\"'; + ret[2] = '\0'; + return ret; + } + ret = (char *) malloc(alloc_len + 1); dest = ret; - *dest++ = '"'; for (ts = s; *ts != '\0'; ts++) { - if (*ts == '"' || *ts == '\\') { + if (*ts == ' ' || *ts == '"' || *ts == '\\' || *ts == '(' || *ts == ')') { *dest++ = '\\'; } *dest++ = *ts; } - *dest++ = '"'; *dest++ = '\0'; return ret; @@ -680,9 +687,9 @@ static int logcat(transport_type transport, char* serial, int argc, char **argv) char *quoted; log_tags = getenv("ANDROID_LOG_TAGS"); - quoted = escape_argv(log_tags == NULL ? "" : log_tags); + quoted = escape_arg(log_tags == NULL ? "" : log_tags); snprintf(buf, sizeof(buf), - "shell:export ANDROID_LOG_TAGS=%s ; exec logcat", quoted); + "shell:export ANDROID_LOG_TAGS=\"%s\"; exec logcat", quoted); free(quoted); if (!strcmp(argv[0], "longcat")) { @@ -692,7 +699,7 @@ static int logcat(transport_type transport, char* serial, int argc, char **argv) argc -= 1; argv += 1; while(argc-- > 0) { - quoted = escape_argv(*argv++); + quoted = escape_arg(*argv++); strncat(buf, " ", sizeof(buf) - 1); strncat(buf, quoted, sizeof(buf) - 1); free(quoted); @@ -1206,7 +1213,7 @@ top: argc -= 2; argv += 2; while (argc-- > 0) { - char *quoted = escape_argv(*argv++); + char *quoted = escape_arg(*argv++); strncat(buf, " ", sizeof(buf) - 1); strncat(buf, quoted, sizeof(buf) - 1); free(quoted); @@ -1249,7 +1256,7 @@ top: argc -= 2; argv += 2; while (argc-- > 0) { - char *quoted = escape_argv(*argv++); + char *quoted = escape_arg(*argv++); strncat(buf, " ", sizeof(buf) - 1); strncat(buf, quoted, sizeof(buf) - 1); free(quoted); @@ -1674,7 +1681,7 @@ static int pm_command(transport_type transport, char* serial, snprintf(buf, sizeof(buf), "shell:pm"); while(argc-- > 0) { - char *quoted = escape_argv(*argv++); + char *quoted = escape_arg(*argv++); strncat(buf, " ", sizeof(buf) - 1); strncat(buf, quoted, sizeof(buf) - 1); free(quoted); @@ -1708,7 +1715,7 @@ static int delete_file(transport_type transport, char* serial, char* filename) char* quoted; snprintf(buf, sizeof(buf), "shell:rm "); - quoted = escape_argv(filename); + quoted = escape_arg(filename); strncat(buf, quoted, sizeof(buf)-1); free(quoted);