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 <ferivoz@riseup.net>
cute-signatures
Timo Teräs 2021-07-26 10:15:55 +03:00
parent 90228c4d26
commit 62e1cba691
3 changed files with 58 additions and 37 deletions

View File

@ -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;
}

View File

@ -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);

View File

@ -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)))