From 62e1cba691fa101e94d23728022bfd8353947c50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20Ter=C3=A4s?= Date: Mon, 26 Jul 2021 10:15:55 +0300 Subject: [PATCH] adb: adb_walk_adb fix out of boundary write If a signature is longer than max allowed adb signature length then adb_walk_block writes out of boundary of stack variable tmp. The len += snprintf is not safe per standard snprintf implementation (kernel does it differently). Introduce and use apk_blob_push_fmt which does the checking better. Fixes #10752 Reported-by: Samanta Navarro --- src/adb_walk_adb.c | 31 +++++++++++------------ src/apk_blob.h | 2 ++ src/blob.c | 62 ++++++++++++++++++++++++++++++---------------- 3 files changed, 58 insertions(+), 37 deletions(-) diff --git a/src/adb_walk_adb.c b/src/adb_walk_adb.c index 9a74e7e..1127487 100644 --- a/src/adb_walk_adb.c +++ b/src/adb_walk_adb.c @@ -110,14 +110,14 @@ static int adb_walk_block(struct adb *db, struct adb_block *b, struct apk_istrea { struct adb_walk_ctx *ctx = container_of(db, struct adb_walk_ctx, db); struct adb_walk *d = ctx->d; - char tmp[16+ADB_MAX_SIGNATURE_LEN*2]; + char tmp[160]; struct adb_hdr *hdr; struct adb_sign_hdr *s; uint32_t schema_magic = ctx->db.schema; const struct adb_db_schema *ds; - int r, len; size_t sz = adb_block_length(b); - apk_blob_t data; + apk_blob_t data, c = APK_BLOB_BUF(tmp); + int r; switch (adb_block_type(b)) { case ADB_BLOCK_ADB: @@ -126,30 +126,29 @@ static int adb_walk_block(struct adb *db, struct adb_block *b, struct apk_istrea if (ds->magic == schema_magic) break; hdr = apk_istream_peek(is, sizeof *hdr); if (IS_ERR(hdr)) return PTR_ERR(hdr); - len = snprintf(tmp, sizeof tmp, "ADB block, size: %zu, compat: %d, ver: %d", + apk_blob_push_fmt(&c, "ADB block, size: %zu, compat: %d, ver: %d", sz, hdr->adb_compat_ver, hdr->adb_ver); - d->ops->comment(d, APK_BLOB_PTR_LEN(tmp, len)); + d->ops->comment(d, apk_blob_pushed(APK_BLOB_BUF(tmp), c)); if (ds->root && hdr->adb_compat_ver == 0) dump_object(ctx, ds->root, adb_r_root(db)); - break; + return 0; case ADB_BLOCK_SIG: s = (struct adb_sign_hdr*) apk_istream_get(is, sz); data = APK_BLOB_PTR_LEN((char*)s, sz); r = adb_trust_verify_signature(ctx->trust, db, &ctx->vfy, data); - len = snprintf(tmp, sizeof tmp, "sig v%02x h%02x ", s->sign_ver, s->hash_alg); - for (size_t j = sizeof *s; j < data.len; j++) - len += snprintf(&tmp[len], sizeof tmp - len, "%02x", (uint8_t)data.ptr[j]); - len += snprintf(&tmp[len], sizeof tmp - len, ": %s", r ? apk_error_str(r) : "OK"); - d->ops->comment(d, APK_BLOB_PTR_LEN(tmp, len)); + apk_blob_push_fmt(&c, "sig v%02x h%02x ", s->sign_ver, s->hash_alg); + for (size_t j = sizeof *s; j < data.len && c.len > 40; j++) + apk_blob_push_fmt(&c, "%02x", (uint8_t)data.ptr[j]); + if (c.len <= 40) apk_blob_push_blob(&c, APK_BLOB_STRLIT("..")); + apk_blob_push_fmt(&c, ": %s", r ? apk_error_str(r) : "OK"); break; case ADB_BLOCK_DATA: - len = snprintf(tmp, sizeof tmp, "data block, size: %zu", sz); - d->ops->comment(d, APK_BLOB_PTR_LEN(tmp, len)); + apk_blob_push_fmt(&c, "data block, size: %zu", sz); break; default: - len = snprintf(tmp, sizeof tmp, "unknown block %d, size: %zu", - adb_block_type(b), sz); - d->ops->comment(d, APK_BLOB_PTR_LEN(tmp, len)); + apk_blob_push_fmt(&c, "unknown block %d, size: %zu", adb_block_type(b), sz); + break; } + d->ops->comment(d, apk_blob_pushed(APK_BLOB_BUF(tmp), c)); return 0; } diff --git a/src/apk_blob.h b/src/apk_blob.h index 2220a75..97f5503 100644 --- a/src/apk_blob.h +++ b/src/apk_blob.h @@ -116,6 +116,8 @@ void apk_blob_push_uint(apk_blob_t *to, unsigned int value, int radix); void apk_blob_push_csum(apk_blob_t *to, struct apk_checksum *csum); void apk_blob_push_base64(apk_blob_t *to, apk_blob_t binary); void apk_blob_push_hexdump(apk_blob_t *to, apk_blob_t binary); +void apk_blob_push_fmt(apk_blob_t *to, const char *fmt, ...) + __attribute__ ((format (printf, 2, 3))); void apk_blob_pull_char(apk_blob_t *b, int expected); unsigned int apk_blob_pull_uint(apk_blob_t *b, int radix); diff --git a/src/blob.c b/src/blob.c index 32cd92e..2052e8e 100644 --- a/src/blob.c +++ b/src/blob.c @@ -383,27 +383,6 @@ void apk_blob_push_csum(apk_blob_t *to, struct apk_checksum *csum) } } -void apk_blob_push_hexdump(apk_blob_t *to, apk_blob_t binary) -{ - char *d; - int i; - - if (unlikely(APK_BLOB_IS_NULL(*to))) - return; - - if (unlikely(to->len < binary.len * 2)) { - *to = APK_BLOB_NULL; - return; - } - - for (i = 0, d = to->ptr; i < binary.len; i++) { - *(d++) = xd[(binary.ptr[i] >> 4) & 0xf]; - *(d++) = xd[binary.ptr[i] & 0xf]; - } - to->ptr = d; - to->len -= binary.len * 2; -} - static const char b64encode[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; @@ -440,6 +419,47 @@ void apk_blob_push_base64(apk_blob_t *to, apk_blob_t binary) to->len -= needed; } +void apk_blob_push_hexdump(apk_blob_t *to, apk_blob_t binary) +{ + char *d; + int i; + + if (unlikely(APK_BLOB_IS_NULL(*to))) + return; + + if (unlikely(to->len < binary.len * 2)) { + *to = APK_BLOB_NULL; + return; + } + + for (i = 0, d = to->ptr; i < binary.len; i++) { + *(d++) = xd[(binary.ptr[i] >> 4) & 0xf]; + *(d++) = xd[binary.ptr[i] & 0xf]; + } + to->ptr = d; + to->len -= binary.len * 2; +} + +void apk_blob_push_fmt(apk_blob_t *to, const char *fmt, ...) +{ + va_list va; + int n; + + if (unlikely(APK_BLOB_IS_NULL(*to))) + return; + + va_start(va, fmt); + n = vsnprintf(to->ptr, to->len, fmt, va); + va_end(va); + + if (n >= 0 && n <= to->len) { + to->ptr += n; + to->len -= n; + } else { + *to = APK_BLOB_NULL; + } +} + void apk_blob_pull_char(apk_blob_t *b, int expected) { if (unlikely(APK_BLOB_IS_NULL(*b)))