From 51da57d5e6965194b454b63d8f08d9c7e3b747d6 Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Wed, 13 Sep 2017 17:14:51 +0200 Subject: [PATCH] validate JSON before loading This commit also introduces slurp() which reads a file in its entirety. Using this function instead of doing IO in the functions in load_layout.c again and again makes the code cleaner (fixing at least two memory leaks) and avoids re-reading the same file 3 times. related to #2755 --- include/load_layout.h | 10 +++- include/util.h | 8 +++ src/commands.c | 24 +++++++-- src/load_layout.c | 120 +++++++++++++++++------------------------- src/tree.c | 20 +++++-- src/util.c | 32 +++++++++++ 6 files changed, 131 insertions(+), 83 deletions(-) diff --git a/include/load_layout.h b/include/load_layout.h index 0dd81318..9205800f 100644 --- a/include/load_layout.h +++ b/include/load_layout.h @@ -31,6 +31,12 @@ typedef enum { * determine whether the file contains workspaces or regular containers, which * is important to know when deciding where (and how) to append the contents. * */ -json_content_t json_determine_content(const char *filename); +json_content_t json_determine_content(const char *buf, const size_t len); -void tree_append_json(Con *con, const char *filename, char **errormsg); +/** + * Returns true if the provided JSON could be parsed by yajl. + * + */ +bool json_validate(const char *buf, const size_t len); + +void tree_append_json(Con *con, const char *buf, const size_t len, char **errormsg); diff --git a/include/util.h b/include/util.h index 6c5fc4c6..de6fa568 100644 --- a/include/util.h +++ b/include/util.h @@ -164,3 +164,11 @@ void kill_nagbar(pid_t *nagbar_pid, bool wait_for_it); * if the number could be parsed. */ bool parse_long(const char *str, long *out, int base); + +/** + * Slurp reads path in its entirety into buf, returning the length of the file + * or -1 if the file could not be read. buf is set to a buffer of appropriate + * size, or NULL if -1 is returned. + * + */ +ssize_t slurp(const char *path, char **buf); diff --git a/src/commands.c b/src/commands.c index faee3916..2697d6e1 100644 --- a/src/commands.c +++ b/src/commands.c @@ -773,13 +773,25 @@ void cmd_append_layout(I3_CMD, const char *cpath) { /* Make sure we allow paths like '~/.i3/layout.json' */ path = resolve_tilde(path); - json_content_t content = json_determine_content(path); + char *buf = NULL; + ssize_t len; + if ((len = slurp(path, &buf)) < 0) { + /* slurp already logged an error. */ + goto out; + } + + if (!json_validate(buf, len)) { + ELOG("Could not parse \"%s\" as JSON, not loading.\n", path); + yerror("Could not parse \"%s\" as JSON.", path); + goto out; + } + + json_content_t content = json_determine_content(buf, len); LOG("JSON content = %d\n", content); if (content == JSON_CONTENT_UNKNOWN) { ELOG("Could not determine the contents of \"%s\", not loading.\n", path); yerror("Could not determine the contents of \"%s\".", path); - free(path); - return; + goto out; } Con *parent = focused; @@ -795,7 +807,7 @@ void cmd_append_layout(I3_CMD, const char *cpath) { } DLOG("Appending to parent=%p instead of focused=%p\n", parent, focused); char *errormsg = NULL; - tree_append_json(parent, path, &errormsg); + tree_append_json(parent, buf, len, &errormsg); if (errormsg != NULL) { yerror(errormsg); free(errormsg); @@ -820,8 +832,10 @@ void cmd_append_layout(I3_CMD, const char *cpath) { if (content == JSON_CONTENT_WORKSPACE) ipc_send_workspace_event("restored", parent, NULL); - free(path); cmd_output->needs_tree_render = true; +out: + free(path); + free(buf); } /* diff --git a/src/load_layout.c b/src/load_layout.c index 0fa3e85b..071b3ccd 100644 --- a/src/load_layout.c +++ b/src/load_layout.c @@ -69,9 +69,9 @@ static int json_start_map(void *ctx) { json_node->name = NULL; json_node->parent = parent; } - /* json_node is incomplete and should be removed if parsing fails */ - incomplete++; - DLOG("incomplete = %d\n", incomplete); + /* json_node is incomplete and should be removed if parsing fails */ + incomplete++; + DLOG("incomplete = %d\n", incomplete); } } return 1; @@ -170,8 +170,8 @@ static int json_end_map(void *ctx) { LOG("Creating window\n"); x_con_init(json_node); json_node = json_node->parent; - incomplete--; - DLOG("incomplete = %d\n", incomplete); + incomplete--; + DLOG("incomplete = %d\n", incomplete); } if (parsing_swallows && swallow_is_empty) { @@ -538,36 +538,42 @@ static int json_determine_content_string(void *ctx, const unsigned char *val, si return 0; } +/* + * Returns true if the provided JSON could be parsed by yajl. + * + */ +bool json_validate(const char *buf, const size_t len) { + bool valid = true; + yajl_handle hand = yajl_alloc(NULL, NULL, NULL); + /* Allowing comments allows for more user-friendly layout files. */ + yajl_config(hand, yajl_allow_comments, true); + /* Allow multiple values, i.e. multiple nodes to attach */ + yajl_config(hand, yajl_allow_multiple_values, true); + + setlocale(LC_NUMERIC, "C"); + if (yajl_parse(hand, (const unsigned char *)buf, len) != yajl_status_ok) { + unsigned char *str = yajl_get_error(hand, 1, (const unsigned char *)buf, len); + ELOG("JSON parsing error: %s\n", str); + yajl_free_error(hand, str); + valid = false; + } + setlocale(LC_NUMERIC, ""); + + yajl_complete_parse(hand); + yajl_free(hand); + + return valid; +} + /* Parses the given JSON file until it encounters the first “type” property to * determine whether the file contains workspaces or regular containers, which * is important to know when deciding where (and how) to append the contents. * */ -json_content_t json_determine_content(const char *filename) { - FILE *f; - if ((f = fopen(filename, "r")) == NULL) { - ELOG("Cannot open file \"%s\"\n", filename); - return JSON_CONTENT_UNKNOWN; - } - struct stat stbuf; - if (fstat(fileno(f), &stbuf) != 0) { - ELOG("Cannot fstat() \"%s\"\n", filename); - fclose(f); - return JSON_CONTENT_UNKNOWN; - } - char *buf = smalloc(stbuf.st_size); - int n = fread(buf, 1, stbuf.st_size, f); - if (n != stbuf.st_size) { - ELOG("File \"%s\" could not be read entirely, not loading.\n", filename); - fclose(f); - return JSON_CONTENT_UNKNOWN; - } - DLOG("read %d bytes\n", n); +json_content_t json_determine_content(const char *buf, const size_t len) { // We default to JSON_CONTENT_CON because it is legal to not include // “"type": "con"” in the JSON files for better readability. content_result = JSON_CONTENT_CON; content_level = 0; - yajl_gen g; - yajl_handle hand; static yajl_callbacks callbacks = { .yajl_string = json_determine_content_string, .yajl_map_key = json_key, @@ -576,51 +582,27 @@ json_content_t json_determine_content(const char *filename) { .yajl_end_map = json_determine_content_shallower, .yajl_end_array = json_determine_content_shallower, }; - g = yajl_gen_alloc(NULL); - hand = yajl_alloc(&callbacks, NULL, (void *)g); + yajl_handle hand = yajl_alloc(&callbacks, NULL, NULL); /* Allowing comments allows for more user-friendly layout files. */ yajl_config(hand, yajl_allow_comments, true); /* Allow multiple values, i.e. multiple nodes to attach */ yajl_config(hand, yajl_allow_multiple_values, true); - yajl_status stat; setlocale(LC_NUMERIC, "C"); - stat = yajl_parse(hand, (const unsigned char *)buf, n); + const yajl_status stat = yajl_parse(hand, (const unsigned char *)buf, len); if (stat != yajl_status_ok && stat != yajl_status_client_canceled) { - unsigned char *str = yajl_get_error(hand, 1, (const unsigned char *)buf, n); + unsigned char *str = yajl_get_error(hand, 1, (const unsigned char *)buf, len); ELOG("JSON parsing error: %s\n", str); yajl_free_error(hand, str); } setlocale(LC_NUMERIC, ""); yajl_complete_parse(hand); - - fclose(f); + yajl_free(hand); return content_result; } -void tree_append_json(Con *con, const char *filename, char **errormsg) { - FILE *f; - if ((f = fopen(filename, "r")) == NULL) { - ELOG("Cannot open file \"%s\"\n", filename); - return; - } - struct stat stbuf; - if (fstat(fileno(f), &stbuf) != 0) { - ELOG("Cannot fstat() \"%s\"\n", filename); - fclose(f); - return; - } - char *buf = smalloc(stbuf.st_size); - int n = fread(buf, 1, stbuf.st_size, f); - if (n != stbuf.st_size) { - ELOG("File \"%s\" could not be read entirely, not loading.\n", filename); - fclose(f); - return; - } - DLOG("read %d bytes\n", n); - yajl_gen g; - yajl_handle hand; +void tree_append_json(Con *con, const char *buf, const size_t len, char **errormsg) { static yajl_callbacks callbacks = { .yajl_boolean = json_bool, .yajl_integer = json_int, @@ -631,13 +613,11 @@ void tree_append_json(Con *con, const char *filename, char **errormsg) { .yajl_end_map = json_end_map, .yajl_end_array = json_end_array, }; - g = yajl_gen_alloc(NULL); - hand = yajl_alloc(&callbacks, NULL, (void *)g); + yajl_handle hand = yajl_alloc(&callbacks, NULL, NULL); /* Allowing comments allows for more user-friendly layout files. */ yajl_config(hand, yajl_allow_comments, true); /* Allow multiple values, i.e. multiple nodes to attach */ yajl_config(hand, yajl_allow_multiple_values, true); - yajl_status stat; json_node = con; to_focus = NULL; incomplete = 0; @@ -649,19 +629,19 @@ void tree_append_json(Con *con, const char *filename, char **errormsg) { parsing_focus = false; parsing_marks = false; setlocale(LC_NUMERIC, "C"); - stat = yajl_parse(hand, (const unsigned char *)buf, n); + const yajl_status stat = yajl_parse(hand, (const unsigned char *)buf, len); if (stat != yajl_status_ok) { - unsigned char *str = yajl_get_error(hand, 1, (const unsigned char *)buf, n); + unsigned char *str = yajl_get_error(hand, 1, (const unsigned char *)buf, len); ELOG("JSON parsing error: %s\n", str); if (errormsg != NULL) *errormsg = sstrdup((const char *)str); yajl_free_error(hand, str); - while (incomplete-- > 0) { - Con *parent = json_node->parent; - DLOG("freeing incomplete container %p\n", json_node); - con_free(json_node); - json_node = parent; - } + while (incomplete-- > 0) { + Con *parent = json_node->parent; + DLOG("freeing incomplete container %p\n", json_node); + con_free(json_node); + json_node = parent; + } } /* In case not all containers were restored, we need to fix the @@ -672,10 +652,8 @@ void tree_append_json(Con *con, const char *filename, char **errormsg) { setlocale(LC_NUMERIC, ""); yajl_complete_parse(hand); yajl_free(hand); - yajl_gen_free(g); - fclose(f); - free(buf); - if (to_focus) + if (to_focus) { con_focus(to_focus); + } } diff --git a/src/tree.c b/src/tree.c index 296a6a37..b3d2ce93 100644 --- a/src/tree.c +++ b/src/tree.c @@ -64,12 +64,19 @@ static Con *_create___i3(void) { * */ bool tree_restore(const char *path, xcb_get_geometry_reply_t *geometry) { + bool result = false; char *globbed = resolve_tilde(path); if (!path_exists(globbed)) { LOG("%s does not exist, not restoring tree\n", globbed); - free(globbed); - return false; + goto out; + } + + char *buf = NULL; + ssize_t len; + if ((len = slurp(globbed, &buf)) < 0) { + /* slurp already logged an error. */ + goto out; } /* TODO: refactor the following */ @@ -81,8 +88,7 @@ bool tree_restore(const char *path, xcb_get_geometry_reply_t *geometry) { geometry->height}; focused = croot; - tree_append_json(focused, globbed, NULL); - free(globbed); + tree_append_json(focused, buf, len, NULL); DLOG("appended tree, using new root\n"); croot = TAILQ_FIRST(&(croot->nodes_head)); @@ -104,8 +110,12 @@ bool tree_restore(const char *path, xcb_get_geometry_reply_t *geometry) { } restore_open_placeholder_windows(croot); + result = true; - return true; +out: + free(globbed); + free(buf); + return result; } /* diff --git a/src/util.c b/src/util.c index 32c3c57e..cd5ee03e 100644 --- a/src/util.c +++ b/src/util.c @@ -475,3 +475,35 @@ bool parse_long(const char *str, long *out, int base) { *out = result; return true; } + +/* + * Slurp reads path in its entirety into buf, returning the length of the file + * or -1 if the file could not be read. buf is set to a buffer of appropriate + * size, or NULL if -1 is returned. + * + */ +ssize_t slurp(const char *path, char **buf) { + FILE *f; + if ((f = fopen(path, "r")) == NULL) { + ELOG("Cannot open file \"%s\": %s\n", path, strerror(errno)); + return -1; + } + struct stat stbuf; + if (fstat(fileno(f), &stbuf) != 0) { + ELOG("Cannot fstat() \"%s\": %s\n", path, strerror(errno)); + fclose(f); + return -1; + } + /* Allocate one extra NUL byte to make the buffer usable with C string + * functions. yajl doesn’t need this, but this makes slurp safer. */ + *buf = scalloc(stbuf.st_size + 1, 1); + size_t n = fread(*buf, 1, stbuf.st_size, f); + fclose(f); + if ((ssize_t)n != stbuf.st_size) { + ELOG("File \"%s\" could not be read entirely: got %zd, want %zd\n", path, n, stbuf.st_size); + free(buf); + *buf = NULL; + return -1; + } + return (ssize_t)n; +} -- 2.39.5