From 1f9edb0b84fb0d6175d81cd06bc37fcedf809f86 Mon Sep 17 00:00:00 2001 From: Alexandre Perrin Date: Wed, 7 Jun 2017 19:37:14 +0200 Subject: [PATCH] Fix a potential use-after-free on quirc_resize failure. Before this patch, in the event of `q->image` reallocation success but `q->pixels` failure, `q->image` would point to a freed memory. After this patch, once quirc_resize() returns, `q->image` consistently point to a memory address that can be freed. There is still an inconsistency left in the example codepath: `q->{h,w}` would hold the "old" values, while `q->image` allocated size would be based in the requested width/height; hence the comment update about the state of the QR-code recognizer (it should only be passed to quirc_destroy()). --- lib/quirc.c | 12 +++--------- lib/quirc.h | 3 ++- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/lib/quirc.c b/lib/quirc.c index 4eabe81..2c6192f 100644 --- a/lib/quirc.c +++ b/lib/quirc.c @@ -36,8 +36,7 @@ struct quirc *quirc_new(void) void quirc_destroy(struct quirc *q) { - if (q->image) - free(q->image); + free(q->image); if (sizeof(*q->image) != sizeof(*q->pixels)) free(q->pixels); @@ -47,23 +46,18 @@ void quirc_destroy(struct quirc *q) int quirc_resize(struct quirc *q, int w, int h) { uint8_t *new_image = realloc(q->image, w * h); - if (!new_image) return -1; + q->image = new_image; if (sizeof(*q->image) != sizeof(*q->pixels)) { size_t new_size = w * h * sizeof(quirc_pixel_t); quirc_pixel_t *new_pixels = realloc(q->pixels, new_size); - - if (!new_pixels) { - free(new_image); + if (!new_pixels) return -1; - } - q->pixels = new_pixels; } - q->image = new_image; q->w = w; q->h = h; diff --git a/lib/quirc.h b/lib/quirc.h index 0e7cb94..47adf4b 100644 --- a/lib/quirc.h +++ b/lib/quirc.h @@ -40,7 +40,8 @@ void quirc_destroy(struct quirc *q); * specified before codes can be analyzed. * * This function returns 0 on success, or -1 if sufficient memory could - * not be allocated. + * not be allocated. On failure the QR-code recognizer should not be + * used and is expected to be given to quirc_destroy() for cleanup. */ int quirc_resize(struct quirc *q, int w, int h);