From adfcdb050c59b2a33b108cec6fc8d94c0c7aff0b Mon Sep 17 00:00:00 2001 From: Tobias Schramm Date: Tue, 20 Feb 2018 12:03:25 +0100 Subject: [PATCH 1/6] autoupdater: Add safe_malloc function safe_malloc is a wrapper around malloc that aborts the current process if the memory allocation fails Signed-off-by: Tobias Schramm --- admin/autoupdater/src/util.c | 12 ++++++++++++ admin/autoupdater/src/util.h | 1 + 2 files changed, 13 insertions(+) diff --git a/admin/autoupdater/src/util.c b/admin/autoupdater/src/util.c index 881523f..1221925 100644 --- a/admin/autoupdater/src/util.c +++ b/admin/autoupdater/src/util.c @@ -100,3 +100,15 @@ float get_uptime(void) { fputs("autoupdater: error: unable to determine uptime\n", stderr); exit(1); } + +void *safe_malloc(size_t size, char *errmsg) { + void *ret = malloc(size); + + if (ret) + return ret; + + if (errmsg) + fprintf(stderr, "autoupdater: error: %s\n", errmsg); + + abort(); +} diff --git a/admin/autoupdater/src/util.h b/admin/autoupdater/src/util.h index 5c23d79..65c0061 100644 --- a/admin/autoupdater/src/util.h +++ b/admin/autoupdater/src/util.h @@ -28,3 +28,4 @@ void run_dir(const char *dir); void randomize(void); float get_uptime(void); +void *safe_malloc(size_t size, char *errmsg); From c52c3495e774ba63982c82b67e937cea6df26a18 Mon Sep 17 00:00:00 2001 From: Tobias Schramm Date: Tue, 20 Feb 2018 12:06:44 +0100 Subject: [PATCH 2/6] autoupdater: Allocate signature using safe_malloc Previously the signature buffer could have been a null ptr leading to a null ptr dereference Signed-off-by: Tobias Schramm --- admin/autoupdater/src/manifest.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/admin/autoupdater/src/manifest.c b/admin/autoupdater/src/manifest.c index 0c51c24..dbad89e 100644 --- a/admin/autoupdater/src/manifest.c +++ b/admin/autoupdater/src/manifest.c @@ -23,9 +23,9 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ - #include "hexutil.h" #include "manifest.h" +#include "util.h" #include #include @@ -80,7 +80,8 @@ static bool parse_rfc3339(const char *input, time_t *date) { void parse_line(char *line, struct manifest *m, const char *branch, const char *image_name) { if (m->sep_found) { - ecdsa_signature_t *sig = malloc(sizeof(ecdsa_signature_t)); + ecdsa_signature_t *sig = safe_malloc(sizeof(ecdsa_signature_t), "failed to allocate memory for signature"); + if (!parsehex(sig, line, sizeof(*sig))) { free(sig); fprintf(stderr, "autoupdater: warning: garbage in signature area: %s\n", line); From c9b1b760346e0c90eb702b9119ea1d6915ce39fb Mon Sep 17 00:00:00 2001 From: Tobias Schramm Date: Tue, 20 Feb 2018 12:09:15 +0100 Subject: [PATCH 3/6] autoupdater: Fix possible null ptr on signature list Previously reallocation of the signature list could fail silently leading to a null ptr dereference Signed-off-by: Tobias Schramm --- admin/autoupdater/src/manifest.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/admin/autoupdater/src/manifest.c b/admin/autoupdater/src/manifest.c index dbad89e..16fcdc6 100644 --- a/admin/autoupdater/src/manifest.c +++ b/admin/autoupdater/src/manifest.c @@ -89,6 +89,11 @@ void parse_line(char *line, struct manifest *m, const char *branch, const char * } m->n_signatures++; m->signatures = realloc(m->signatures, m->n_signatures * sizeof(ecdsa_signature_t *)); + if (!m->signatures) { + fprintf(stderr, "autoupdater: error: failed to extend signature list\n"); + abort(); + } + m->signatures[m->n_signatures - 1] = sig; } else if (strcmp(line, "---") == 0) { m->sep_found = true; From a96a2d7718152afeb22f5e6aa4b6fd1bb7735992 Mon Sep 17 00:00:00 2001 From: Tobias Schramm Date: Tue, 20 Feb 2018 12:12:50 +0100 Subject: [PATCH 4/6] autoupdater: Replace mallocs with safe_malloc During parsing of cinfiguration multiple null ptr dereferences were possible due to a lack of NULL checks Signed-off-by: Tobias Schramm --- admin/autoupdater/src/settings.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/admin/autoupdater/src/settings.c b/admin/autoupdater/src/settings.c index cc9d17a..50a7afb 100644 --- a/admin/autoupdater/src/settings.c +++ b/admin/autoupdater/src/settings.c @@ -27,6 +27,7 @@ #include "settings.h" #include "hexutil.h" +#include "util.h" #include @@ -97,7 +98,7 @@ static const char ** load_string_list(struct uci_context *ctx, struct uci_sectio i++; *len = i; - const char **ret = malloc(i * sizeof(char *)); + const char **ret = safe_malloc(i * sizeof(char *), "failed to allocate string list"); i = 0; uci_foreach_element(&o->v.list, e) @@ -154,7 +155,7 @@ void load_settings(struct settings *settings) { settings->mirrors = load_string_list(ctx, branch, "mirror", &settings->n_mirrors); const char **pubkeys_str = load_string_list(ctx, branch, "pubkey", &settings->n_pubkeys); - settings->pubkeys = malloc(settings->n_pubkeys * sizeof(ecc_25519_work_t)); + settings->pubkeys = safe_malloc(settings->n_pubkeys * sizeof(ecc_25519_work_t), "failed to allocate memory for public keys"); size_t ignored_keys = 0; for (size_t i = 0; i < settings->n_pubkeys; i++) { ecc_int256_t pubkey_packed; From a6336d0bce39e01bcd44a194ed557a6ca167be03 Mon Sep 17 00:00:00 2001 From: Tobias Schramm Date: Tue, 20 Feb 2018 12:15:42 +0100 Subject: [PATCH 5/6] autoupdater: Check if allocation of uci contect was successfull Previously the return value of uci_alloc_context was not checked leading to a possible null ptr dereference Signed-off-by: Tobias Schramm --- admin/autoupdater/src/settings.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/admin/autoupdater/src/settings.c b/admin/autoupdater/src/settings.c index 50a7afb..47cc8f6 100644 --- a/admin/autoupdater/src/settings.c +++ b/admin/autoupdater/src/settings.c @@ -110,6 +110,11 @@ static const char ** load_string_list(struct uci_context *ctx, struct uci_sectio void load_settings(struct settings *settings) { struct uci_context *ctx = uci_alloc_context(); + if (!ctx) { + fprintf(stderr, "autoupdater: error: failed to allocate uci context\n"); + exit(1); + } + ctx->flags &= ~UCI_FLAG_STRICT; struct uci_package *p; From 5f65095e47a844de67bbd01fe1122a7c116f3ed9 Mon Sep 17 00:00:00 2001 From: Tobias Schramm Date: Tue, 20 Feb 2018 12:17:49 +0100 Subject: [PATCH 6/6] autoupdater: Replace malloc by safe_malloc During argument parsing the allocation of the string array for the mirror list had no NULL check leading to a possible null ptr dereference Signed-off-by: Tobias Schramm --- admin/autoupdater/src/autoupdater.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/admin/autoupdater/src/autoupdater.c b/admin/autoupdater/src/autoupdater.c index 0c08f3a..5db868b 100644 --- a/admin/autoupdater/src/autoupdater.c +++ b/admin/autoupdater/src/autoupdater.c @@ -144,7 +144,8 @@ static void parse_args(int argc, char *argv[], struct settings *settings) { if (optind < argc) { settings->n_mirrors = argc - optind; - settings->mirrors = malloc(settings->n_mirrors * sizeof(char *)); + settings->mirrors = safe_malloc(settings->n_mirrors * sizeof(char *), "failed to allocate memory for mirror list"); + for (int i = optind; i < argc; i++) { settings->mirrors[i - optind] = argv[i]; }