From 4b54bd3cb36738ed0d9605e85340946b8097f601 Mon Sep 17 00:00:00 2001 From: MiesSuomesta <32951624+MiesSuomesta@users.noreply.github.com> Date: Thu, 19 Dec 2019 07:38:58 +0200 Subject: [PATCH] Fixes segfaults & securing a bit Fixes some segfaults & tweaks --- json_object.c | 108 ++++++++++++++++++++++++++++++++++---------------- linkhash.c | 2 + printbuf.c | 85 +++++++++++++++++++++++++++++++-------- 3 files changed, 144 insertions(+), 51 deletions(-) diff --git a/json_object.c b/json_object.c index 74d5266..4b5a70b 100644 --- a/json_object.c +++ b/json_object.c @@ -10,6 +10,8 @@ * */ +//#pragma GCC optimize ("O0") + #include "config.h" #include "strerror_override.h" @@ -143,16 +145,13 @@ static int json_escape_str(struct printbuf *pb, const char *str, int len, int fl default: if(c < ' ') { - char sbuf[7]; if(pos - start_offset > 0) printbuf_memappend(pb, str + start_offset, pos - start_offset); - snprintf(sbuf, sizeof(sbuf), - "\\u00%c%c", - json_hex_chars[c >> 4], - json_hex_chars[c & 0xf]); - printbuf_memappend_fast(pb, sbuf, (int) sizeof(sbuf) - 1); + sprintbuf(pb, "\\u00%c%c", + json_hex_chars[c >> 4], + json_hex_chars[c & 0xf]); start_offset = ++pos; } else pos++; @@ -220,14 +219,16 @@ static void json_object_generic_delete(struct json_object* jso) lh_table_delete(json_object_table, jso); #endif /* REFCOUNT_DEBUG */ printbuf_free(jso->_pb); + jso->_pb = NULL; free(jso); + jso= NULL; } static struct json_object* json_object_new(enum json_type o_type) { struct json_object *jso; - jso = (struct json_object*)calloc(sizeof(struct json_object), 1); + jso = (struct json_object*)calloc(1, sizeof(struct json_object)); if (!jso) return NULL; jso->o_type = o_type; @@ -325,24 +326,30 @@ const char* json_object_to_json_string_length(struct json_object *jso, int flags const char *r = NULL; size_t s = 0; - if (!jso) - { - s = 4; - r = "null"; - } - else if ((jso->_pb) || (jso->_pb = printbuf_new())) + /* Failure case set first */ + s = 4; + r = "null"; + + if (jso) { - printbuf_reset(jso->_pb); + if ( jso->_pb == NULL ) + jso->_pb = printbuf_new(); - if(jso->_to_json_string(jso, jso->_pb, 0, flags) >= 0) + if ( jso->_pb != NULL ) { - s = (size_t)jso->_pb->bpos; - r = jso->_pb->buf; + printbuf_reset(jso->_pb); + + if(jso->_to_json_string(jso, jso->_pb, 0, flags) >= 0) + { + s = (size_t)jso->_pb->bpos; + r = jso->_pb->buf; + } } } if (length) *length = s; + return r; } @@ -400,10 +407,12 @@ static int json_object_object_to_json_string(struct json_object* jso, indent(pb, level+1, flags); printbuf_strappend(pb, "\""); json_escape_str(pb, iter.key, strlen(iter.key), flags); + if (flags & JSON_C_TO_STRING_SPACED) printbuf_strappend(pb, "\": "); else printbuf_strappend(pb, "\":"); + if(iter.val == NULL) printbuf_strappend(pb, "null"); else @@ -467,6 +476,19 @@ struct lh_table* json_object_get_object(const struct json_object *jso) } } +static char *stringdup(const char *s) +{ + size_t len = strnlen (s, 1024) + 1; + char *new = malloc (len); + + if (new == NULL) + return NULL; + + new[len] = 0; + return (char *) memcpy (new, s, len); +} + + int json_object_object_add_ex(struct json_object* jso, const char *const key, struct json_object *const val, @@ -481,9 +503,9 @@ int json_object_object_add_ex(struct json_object* jso, // We lookup the entry and replace the value, rather than just deleting // and re-adding it, so the existing key remains valid. hash = lh_get_hash(jso->o.c_object, (const void *)key); - existing_entry = (opts & JSON_C_OBJECT_ADD_KEY_IS_NEW) ? NULL : - lh_table_lookup_entry_w_hash(jso->o.c_object, - (const void *)key, hash); + existing_entry = (opts & JSON_C_OBJECT_ADD_KEY_IS_NEW) ? + NULL : + lh_table_lookup_entry_w_hash(jso->o.c_object, (const void *)key, hash); // The caller must avoid creating loops in the object tree, but do a // quick check anyway to make sure we're not creating a trivial loop. @@ -492,17 +514,26 @@ int json_object_object_add_ex(struct json_object* jso, if (!existing_entry) { - const void *const k = (opts & JSON_C_OBJECT_KEY_IS_CONSTANT) ? - (const void *)key : strdup(key); + char *key_dup = stringdup(key); + + /* key duplicate must be done first */ + const void *const k = ((opts & JSON_C_OBJECT_KEY_IS_CONSTANT) ? (const void *)key : (const void *)key_dup); + + /* key duplicate must be freed here if constant */ + if (opts & JSON_C_OBJECT_KEY_IS_CONSTANT) + free(key_dup); + if (k == NULL) return -1; + return lh_table_insert_w_hash(jso->o.c_object, k, val, hash, opts); + } else { + existing_value = (json_object *) lh_entry_v(existing_entry); + if (existing_value) + json_object_put(existing_value); + existing_entry->v = val; + return 0; } - existing_value = (json_object *) lh_entry_v(existing_entry); - if (existing_value) - json_object_put(existing_value); - existing_entry->v = val; - return 0; } int json_object_object_add(struct json_object* jso, const char *key, @@ -1428,7 +1459,7 @@ static int json_object_deep_copy_recursive(struct json_object *src, struct json_ if (shallow_copy_rc < 1) { errno = EINVAL; - return -1; + return -120; } assert(*dst != NULL); @@ -1442,13 +1473,13 @@ static int json_object_deep_copy_recursive(struct json_object *src, struct json_ else if (json_object_deep_copy_recursive(iter.val, src, iter.key, -1, &jso, shallow_copy) < 0) { json_object_put(jso); - return -1; + return -130; } if (json_object_object_add(*dst, iter.key, jso) < 0) { json_object_put(jso); - return -1; + return -140; } } break; @@ -1464,13 +1495,13 @@ static int json_object_deep_copy_recursive(struct json_object *src, struct json_ else if (json_object_deep_copy_recursive(jso1, src, NULL, ii, &jso, shallow_copy) < 0) { json_object_put(jso); - return -1; + return -150; } if (json_object_array_add(*dst, jso) < 0) { json_object_put(jso); - return -1; + return -160; } } break; @@ -1491,9 +1522,17 @@ int json_object_deep_copy(struct json_object *src, struct json_object **dst, jso int rc; /* Check if arguments are sane ; *dst must not point to a non-NULL object */ - if (!src || !dst || *dst) { + if (!src) { errno = EINVAL; - return -1; + return -101; + } + if (!dst) { + errno = EINVAL; + return -102; + } + if (*dst) { + errno = EINVAL; + return -103; } if (shallow_copy == NULL) @@ -1501,6 +1540,7 @@ int json_object_deep_copy(struct json_object *src, struct json_object **dst, jso rc = json_object_deep_copy_recursive(src, NULL, NULL, -1, dst, shallow_copy); if (rc < 0) { + json_object_put(*dst); *dst = NULL; } diff --git a/linkhash.c b/linkhash.c index f324a10..3e65302 100644 --- a/linkhash.c +++ b/linkhash.c @@ -563,7 +563,9 @@ void lh_table_free(struct lh_table *t) t->free_fn(c); } free(t->table); + t->table = NULL; free(t); + t = NULL; } diff --git a/printbuf.c b/printbuf.c index 6c77b5d..3fb8e4b 100644 --- a/printbuf.c +++ b/printbuf.c @@ -13,6 +13,10 @@ * (http://www.opensource.org/licenses/mit-license.php) */ +#define PRINTBUF_DEBUG 1 + +#pragma GCC optimize ("O0") + #include "config.h" #include @@ -30,6 +34,8 @@ #include "snprintf_compat.h" #include "vasprintf_compat.h" +#define PRINTBUF_DEFAULT_SIZE (256) + static int printbuf_extend(struct printbuf *p, int min_size); struct printbuf* printbuf_new(void) @@ -38,9 +44,10 @@ struct printbuf* printbuf_new(void) p = (struct printbuf*)calloc(1, sizeof(struct printbuf)); if(!p) return NULL; - p->size = 32; + p->size = PRINTBUF_DEFAULT_SIZE; p->bpos = 0; - if(!(p->buf = (char*)malloc(p->size))) { + p->buf = (char*)calloc(1, p->size); + if(p->buf == NULL) { free(p); return NULL; } @@ -59,32 +66,60 @@ struct printbuf* printbuf_new(void) */ static int printbuf_extend(struct printbuf *p, int min_size) { - char *t; +#define PRINTBUF_EXTEND_BY_BYTES_MIN (8) + char *t = NULL; int new_size; - if (p->size >= min_size) + if( + (p != NULL) && + (p->buf != NULL) && + (p->size >= min_size) + ) return 0; new_size = p->size * 2; - if (new_size < min_size + 8) - new_size = min_size + 8; + + if (new_size < (min_size + PRINTBUF_EXTEND_BY_BYTES_MIN)) + new_size = min_size + PRINTBUF_EXTEND_BY_BYTES_MIN; + #ifdef PRINTBUF_DEBUG MC_DEBUG("printbuf_memappend: realloc " "bpos=%d min_size=%d old_size=%d new_size=%d\n", p->bpos, min_size, p->size, new_size); #endif /* PRINTBUF_DEBUG */ - if(!(t = (char*)realloc(p->buf, new_size))) + + if (p != NULL) + { + t = (char*)calloc(1, new_size); + if ( (t != NULL) && + (p->buf != NULL)) + { + memcpy(t, p->buf, p->size); + } + } + + if (t == NULL) return -1; + p->size = new_size; p->buf = t; + return 0; } int printbuf_memappend(struct printbuf *p, const char *buf, int size) { + + if ( (p->size > 0) && (p->buf == NULL)) { + int size_wanted = p->size; + p->size = 0; + if (printbuf_extend(p, size_wanted) < 0) + return -1; + } + if (p->size <= p->bpos + size + 1) { - if (printbuf_extend(p, p->bpos + size + 1) < 0) - return -1; + if (printbuf_extend(p, p->bpos + size + 1) < 0) + return -2; } memcpy(p->buf + p->bpos, buf, size); p->bpos += size; @@ -116,20 +151,21 @@ int sprintbuf(struct printbuf *p, const char *msg, ...) { va_list ap; char *t; - int size; - char buf[128]; + long int size; +#define PRINTBUF_DEFAULT_SIZE_BUF ((PRINTBUF_DEFAULT_SIZE<<2) > +1) + char buf[PRINTBUF_DEFAULT_SIZE_BUF]; /* user stack buffer first */ va_start(ap, msg); - size = vsnprintf(buf, 128, msg, ap); + size = (long int)vsnprintf(buf, sizeof(buf), msg, ap); va_end(ap); /* if string is greater than stack buffer, then use dynamic string with vasprintf. Note: some implementation of vsnprintf return -1 if output is truncated whereas some return the number of bytes that would have been written - this code handles both cases. */ - if(size == -1 || size > 127) { + if(size == -1 || size > (long int)sizeof(buf)) { va_start(ap, msg); - if((size = vasprintf(&t, msg, ap)) < 0) { va_end(ap); return -1; } + if((size = (long int)vasprintf(&t, msg, ap)) < 0) { va_end(ap); return -1; } va_end(ap); printbuf_memappend(p, t, size); free(t); @@ -142,14 +178,29 @@ int sprintbuf(struct printbuf *p, const char *msg, ...) void printbuf_reset(struct printbuf *p) { - p->buf[0] = '\0'; - p->bpos = 0; + if (p != NULL) + { + if ( (p->size > 0) && + (p->buf != NULL) + ) + { + p->buf[0] = '\0'; + } + + p->bpos = 0; + } } void printbuf_free(struct printbuf *p) { if(p) { - free(p->buf); + + if (p->buf != NULL) + free(p->buf); + + p->buf = NULL; + free(p); + p = NULL; } }