Fix adb device ip parsing

The parser assumed that its input was a NUL-terminated string, but it
was not the case: it is just the raw output of "adb devices ip route".

In practice, it was harmless, since the output always ended with '\n'
(which was replaced by '\0' on truncation), but it was incorrect
nonetheless.

Always write a '\0' at the end of the buffer, and explicitly parse as a
NUL-terminated string. For that purpose, avoid the error-prone
sc_str_truncate() util function.
This commit is contained in:
Romain Vimont 2022-02-06 10:52:55 +01:00
parent 5b3ae2cb2f
commit 5d6bd8f9cd
4 changed files with 43 additions and 22 deletions

View file

@ -446,7 +446,7 @@ sc_adb_get_device_ip(struct sc_intr *intr, const char *serial, unsigned flags) {
// "adb shell ip route" output should contain only a few lines // "adb shell ip route" output should contain only a few lines
char buf[1024]; char buf[1024];
ssize_t r = sc_pipe_read_all_intr(intr, pid, pout, buf, sizeof(buf)); ssize_t r = sc_pipe_read_all_intr(intr, pid, pout, buf, sizeof(buf) - 1);
sc_pipe_close(pout); sc_pipe_close(pout);
bool ok = process_check_success_intr(intr, pid, "ip route", flags); bool ok = process_check_success_intr(intr, pid, "ip route", flags);
@ -458,8 +458,8 @@ sc_adb_get_device_ip(struct sc_intr *intr, const char *serial, unsigned flags) {
return NULL; return NULL;
} }
assert((size_t) r <= sizeof(buf)); assert((size_t) r < sizeof(buf));
if (r == sizeof(buf) && buf[sizeof(buf) - 1] != '\0') { if (r == sizeof(buf) - 1) {
// The implementation assumes that the output of "ip route" fits in the // The implementation assumes that the output of "ip route" fits in the
// buffer in a single pass // buffer in a single pass
LOGW("Result of \"ip route\" does not fit in 1Kb. " LOGW("Result of \"ip route\" does not fit in 1Kb. "
@ -467,5 +467,8 @@ sc_adb_get_device_ip(struct sc_intr *intr, const char *serial, unsigned flags) {
return NULL; return NULL;
} }
return sc_adb_parse_device_ip_from_output(buf, r); // It is parsed as a NUL-terminated string
buf[r] = '\0';
return sc_adb_parse_device_ip_from_output(buf);
} }

View file

@ -7,7 +7,7 @@
#include "util/str.h" #include "util/str.h"
static char * static char *
sc_adb_parse_device_ip_from_line(char *line, size_t len) { sc_adb_parse_device_ip_from_line(char *line) {
// One line from "ip route" looks like: // One line from "ip route" looks like:
// "192.168.1.0/24 dev wlan0 proto kernel scope link src 192.168.1.x" // "192.168.1.0/24 dev wlan0 proto kernel scope link src 192.168.1.x"
@ -27,10 +27,12 @@ sc_adb_parse_device_ip_from_line(char *line, size_t len) {
idx_ip += idx_dev_name; idx_ip += idx_dev_name;
char *dev_name = &line[idx_dev_name]; char *dev_name = &line[idx_dev_name];
sc_str_truncate(dev_name, len - idx_dev_name + 1, " \t"); size_t dev_name_len = strcspn(dev_name, " \t");
dev_name[dev_name_len] = '\0';
char *ip = &line[idx_ip]; char *ip = &line[idx_ip];
sc_str_truncate(ip, len - idx_ip + 1, " \t"); size_t ip_len = strcspn(ip, " \t");
ip[ip_len] = '\0';
// Only consider lines where the device name starts with "wlan" // Only consider lines where the device name starts with "wlan"
if (strncmp(dev_name, "wlan", sizeof("wlan") - 1)) { if (strncmp(dev_name, "wlan", sizeof("wlan") - 1)) {
@ -42,23 +44,28 @@ sc_adb_parse_device_ip_from_line(char *line, size_t len) {
} }
char * char *
sc_adb_parse_device_ip_from_output(char *buf, size_t buf_len) { sc_adb_parse_device_ip_from_output(char *str) {
size_t idx_line = 0; size_t idx_line = 0;
while (idx_line < buf_len && buf[idx_line] != '\0') { while (str[idx_line] != '\0') {
char *line = &buf[idx_line]; char *line = &str[idx_line];
size_t len = sc_str_truncate(line, buf_len - idx_line, "\n"); size_t len = strcspn(line, "\n");
// The same, but without any trailing '\r' // The same, but without any trailing '\r'
size_t line_len = sc_str_remove_trailing_cr(line, len); size_t line_len = sc_str_remove_trailing_cr(line, len);
line[line_len] = '\0';
char *ip = sc_adb_parse_device_ip_from_line(line, line_len); char *ip = sc_adb_parse_device_ip_from_line(line);
if (ip) { if (ip) {
// Found // Found
return ip; return ip;
} }
// The next line starts after the '\n' (replaced by `\0`) idx_line += len;
idx_line += len + 1;
if (str[idx_line] != '\0') {
// The next line starts after the '\n'
idx_line += 1;
}
} }
return NULL; return NULL;

View file

@ -8,9 +8,11 @@
/** /**
* Parse the ip from the output of `adb shell ip route` * Parse the ip from the output of `adb shell ip route`
* *
* The parameter must be a NUL-terminated string.
*
* Warning: this function modifies the buffer for optimization purposes. * Warning: this function modifies the buffer for optimization purposes.
*/ */
char * char *
sc_adb_parse_device_ip_from_output(char *buf, size_t buf_len); sc_adb_parse_device_ip_from_output(char *str);
#endif #endif

View file

@ -8,7 +8,7 @@ static void test_get_ip_single_line() {
char ip_route[] = "192.168.1.0/24 dev wlan0 proto kernel scope link src " char ip_route[] = "192.168.1.0/24 dev wlan0 proto kernel scope link src "
"192.168.12.34\r\r\n"; "192.168.12.34\r\r\n";
char *ip = sc_adb_parse_device_ip_from_output(ip_route, sizeof(ip_route)); char *ip = sc_adb_parse_device_ip_from_output(ip_route);
assert(ip); assert(ip);
assert(!strcmp(ip, "192.168.12.34")); assert(!strcmp(ip, "192.168.12.34"));
free(ip); free(ip);
@ -18,7 +18,7 @@ static void test_get_ip_single_line_without_eol() {
char ip_route[] = "192.168.1.0/24 dev wlan0 proto kernel scope link src " char ip_route[] = "192.168.1.0/24 dev wlan0 proto kernel scope link src "
"192.168.12.34"; "192.168.12.34";
char *ip = sc_adb_parse_device_ip_from_output(ip_route, sizeof(ip_route)); char *ip = sc_adb_parse_device_ip_from_output(ip_route);
assert(ip); assert(ip);
assert(!strcmp(ip, "192.168.12.34")); assert(!strcmp(ip, "192.168.12.34"));
free(ip); free(ip);
@ -28,7 +28,7 @@ static void test_get_ip_single_line_with_trailing_space() {
char ip_route[] = "192.168.1.0/24 dev wlan0 proto kernel scope link src " char ip_route[] = "192.168.1.0/24 dev wlan0 proto kernel scope link src "
"192.168.12.34 \n"; "192.168.12.34 \n";
char *ip = sc_adb_parse_device_ip_from_output(ip_route, sizeof(ip_route)); char *ip = sc_adb_parse_device_ip_from_output(ip_route);
assert(ip); assert(ip);
assert(!strcmp(ip, "192.168.12.34")); assert(!strcmp(ip, "192.168.12.34"));
free(ip); free(ip);
@ -40,7 +40,7 @@ static void test_get_ip_multiline_first_ok() {
"10.0.0.0/24 dev rmnet proto kernel scope link src " "10.0.0.0/24 dev rmnet proto kernel scope link src "
"10.0.0.2\r\n"; "10.0.0.2\r\n";
char *ip = sc_adb_parse_device_ip_from_output(ip_route, sizeof(ip_route)); char *ip = sc_adb_parse_device_ip_from_output(ip_route);
assert(ip); assert(ip);
assert(!strcmp(ip, "192.168.1.2")); assert(!strcmp(ip, "192.168.1.2"));
free(ip); free(ip);
@ -52,7 +52,7 @@ static void test_get_ip_multiline_second_ok() {
"192.168.1.0/24 dev wlan0 proto kernel scope link src " "192.168.1.0/24 dev wlan0 proto kernel scope link src "
"192.168.1.3\r\n"; "192.168.1.3\r\n";
char *ip = sc_adb_parse_device_ip_from_output(ip_route, sizeof(ip_route)); char *ip = sc_adb_parse_device_ip_from_output(ip_route);
assert(ip); assert(ip);
assert(!strcmp(ip, "192.168.1.3")); assert(!strcmp(ip, "192.168.1.3"));
free(ip); free(ip);
@ -62,7 +62,15 @@ static void test_get_ip_no_wlan() {
char ip_route[] = "192.168.1.0/24 dev rmnet proto kernel scope link src " char ip_route[] = "192.168.1.0/24 dev rmnet proto kernel scope link src "
"192.168.12.34\r\r\n"; "192.168.12.34\r\r\n";
char *ip = sc_adb_parse_device_ip_from_output(ip_route, sizeof(ip_route)); char *ip = sc_adb_parse_device_ip_from_output(ip_route);
assert(!ip);
}
static void test_get_ip_no_wlan_without_eol() {
char ip_route[] = "192.168.1.0/24 dev rmnet proto kernel scope link src "
"192.168.12.34";
char *ip = sc_adb_parse_device_ip_from_output(ip_route);
assert(!ip); assert(!ip);
} }
@ -70,7 +78,7 @@ static void test_get_ip_truncated() {
char ip_route[] = "192.168.1.0/24 dev rmnet proto kernel scope link src " char ip_route[] = "192.168.1.0/24 dev rmnet proto kernel scope link src "
"\n"; "\n";
char *ip = sc_adb_parse_device_ip_from_output(ip_route, sizeof(ip_route)); char *ip = sc_adb_parse_device_ip_from_output(ip_route);
assert(!ip); assert(!ip);
} }
@ -84,5 +92,6 @@ int main(int argc, char *argv[]) {
test_get_ip_multiline_first_ok(); test_get_ip_multiline_first_ok();
test_get_ip_multiline_second_ok(); test_get_ip_multiline_second_ok();
test_get_ip_no_wlan(); test_get_ip_no_wlan();
test_get_ip_no_wlan_without_eol();
test_get_ip_truncated(); test_get_ip_truncated();
} }