Skip to content

Commit 34e6ee0

Browse files
committed
Improve checking of *out_size in rans4x16 & arith
These should be set to a minimum of at least rans_compress_bound_4x16() or arith_compress_bound() but as they're user supplied parameters we ought to consider the possibility of them being smaller and the potential for an encode failure. (Although arguably it's a user error too as we're not using the API as intended.) This happens in many places, but not consistently when very small sizes are given such that we don't have room for the compression header meta-data.
1 parent 5179428 commit 34e6ee0

File tree

3 files changed

+66
-7
lines changed

3 files changed

+66
-7
lines changed

htscodecs/arith_dynamic.c

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -733,7 +733,7 @@ unsigned char *arith_compress_to(unsigned char *in, unsigned int in_size,
733733
unsigned int c_meta_len;
734734
uint8_t *rle = NULL, *packed = NULL;
735735

736-
if (in_size > INT_MAX) {
736+
if (in_size > INT_MAX || (out && *out_size == 0)) {
737737
*out_size = 0;
738738
return NULL;
739739
}
@@ -751,6 +751,8 @@ unsigned char *arith_compress_to(unsigned char *in, unsigned int in_size,
751751
if (order & X_CAT) {
752752
out[0] = X_CAT;
753753
c_meta_len = 1 + var_put_u32(&out[1], out_end, in_size);
754+
if (c_meta_len + in_size > *out_size)
755+
return NULL;
754756
memcpy(out+c_meta_len, in, in_size);
755757
*out_size = in_size+c_meta_len;
756758
}
@@ -759,6 +761,9 @@ unsigned char *arith_compress_to(unsigned char *in, unsigned int in_size,
759761
int N = (order>>8);
760762
if (N == 0) N = 4; // default for compatibility with old tests
761763

764+
if (N > in_size)
765+
N = in_size;
766+
762767
if (N > 255)
763768
return NULL;
764769

@@ -843,20 +848,26 @@ unsigned char *arith_compress_to(unsigned char *in, unsigned int in_size,
843848
// {1, 128}};
844849

845850
for (j = 1; j <= m[MIN(i,3)][0]; j++) {
851+
if (out2 - out > *out_size)
852+
continue; // an error, but caught in best_sz check later
853+
846854
olen2 = *out_size - (out2 - out);
847855
//fprintf(stderr, "order=%d m=%d\n", order&3, m[MIN(i,4)][j]);
848856
if ((order&3) == 0 && (m[MIN(i,3)][j]&1))
849857
continue;
850858

851859
arith_compress_to(transposed+idx[i], part_len[i],
852860
out2, &olen2, m[MIN(i,3)][j] | X_NOSZ);
853-
if (best_sz > olen2) {
861+
if (olen2 && best_sz > olen2) {
854862
best_sz = olen2;
855863
best_j = j;
856864
}
857865
}
858-
// if (best_j == 0) // none desireable
859-
// return NULL;
866+
867+
if (best_sz == INT_MAX) {
868+
free(transposed);
869+
return NULL;
870+
}
860871
if (best_j != j-1) {
861872
olen2 = *out_size - (out2 - out);
862873
arith_compress_to(transposed+idx[i], part_len[i],
@@ -964,6 +975,12 @@ unsigned char *arith_compress_to(unsigned char *in, unsigned int in_size,
964975
if (*out_size >= in_size) {
965976
out[0] &= ~(3|X_EXT); // no entropy encoding, but keep e.g. PACK
966977
out[0] |= X_CAT | no_size;
978+
979+
if (out + c_meta_len + in_size > out_end) {
980+
free(rle);
981+
free(packed);
982+
return NULL;
983+
}
967984
memcpy(out+c_meta_len, in, in_size);
968985
*out_size = in_size;
969986
}

htscodecs/rANS_static4x16pr.c

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1164,7 +1164,7 @@ void rans_set_cpu(int opts) {
11641164
unsigned char *rans_compress_to_4x16(unsigned char *in, unsigned int in_size,
11651165
unsigned char *out,unsigned int *out_size,
11661166
int order) {
1167-
if (in_size > INT_MAX) {
1167+
if (in_size > INT_MAX || (out && *out_size == 0)) {
11681168
*out_size = 0;
11691169
return NULL;
11701170
}
@@ -1199,6 +1199,12 @@ unsigned char *rans_compress_to_4x16(unsigned char *in, unsigned int in_size,
11991199
int N = (order>>8) & 0xff;
12001200
if (N == 0) N = 4; // default for compatibility with old tests
12011201

1202+
if (N > in_size)
1203+
N = in_size;
1204+
1205+
if (N > 255)
1206+
return NULL;
1207+
12021208
unsigned char *transposed = malloc(in_size);
12031209
unsigned int part_len[256];
12041210
unsigned int idx[256];
@@ -1249,26 +1255,31 @@ unsigned char *rans_compress_to_4x16(unsigned char *in, unsigned int in_size,
12491255
out2_start = out2 = out+7+5*N; // shares a buffer with c_meta
12501256
for (i = 0; i < N; i++) {
12511257
// Brute force try all methods.
1252-
int j, m[] = {1,64,128,0}, best_j = 0, best_sz = in_size+10;
1258+
int j, m[] = {1,64,128,0}, best_j = 0, best_sz = INT_MAX;
12531259
for (j = 0; j < sizeof(m)/sizeof(*m); j++) {
12541260
if ((order & m[j]) != m[j])
12551261
continue;
12561262

12571263
// order-1 *only*; bit check above cannot elide order-0
12581264
if ((order & RANS_ORDER_STRIPE_NO0) && (m[j]&1) == 0)
12591265
continue;
1266+
1267+
if (out2 - out > *out_size)
1268+
continue; // an error, but caught in best_sz check later
1269+
12601270
olen2 = *out_size - (out2 - out);
12611271
rans_compress_to_4x16(transposed+idx[i], part_len[i],
12621272
out2, &olen2,
12631273
m[j] | RANS_ORDER_NOSZ
12641274
| (order&RANS_ORDER_X32));
1265-
if (best_sz > olen2) {
1275+
if (olen2 && best_sz > olen2) {
12661276
best_sz = olen2;
12671277
best_j = j;
12681278
if (j < sizeof(m)/sizeof(*m) && olen2 > out_best_len) {
12691279
unsigned char *tmp = realloc(out_best, olen2);
12701280
if (!tmp) {
12711281
free(out_free);
1282+
free(transposed);
12721283
return NULL;
12731284
}
12741285
out_best = tmp;
@@ -1279,6 +1290,14 @@ unsigned char *rans_compress_to_4x16(unsigned char *in, unsigned int in_size,
12791290
memcpy(out_best, out2, olen2);
12801291
}
12811292
}
1293+
1294+
if (best_sz == INT_MAX) {
1295+
free(out_best);
1296+
free(out_free);
1297+
free(transposed);
1298+
return NULL;
1299+
}
1300+
12821301
if (best_j < sizeof(m)/sizeof(*m)) {
12831302
// Copy the best compression to output buffer if not current
12841303
memcpy(out2, out_best, best_sz);
@@ -1301,6 +1320,9 @@ unsigned char *rans_compress_to_4x16(unsigned char *in, unsigned int in_size,
13011320
out[0] = RANS_ORDER_CAT;
13021321
c_meta_len = 1;
13031322
c_meta_len += var_put_u32(&out[1], out_end, in_size);
1323+
1324+
if (c_meta_len + in_size > *out_size)
1325+
return NULL;
13041326
if (in_size)
13051327
memcpy(out+c_meta_len, in, in_size);
13061328
*out_size = c_meta_len + in_size;
@@ -1404,6 +1426,13 @@ unsigned char *rans_compress_to_4x16(unsigned char *in, unsigned int in_size,
14041426
out[0] &= ~RANS_ORDER_RLE;
14051427
}
14061428

1429+
if (c_meta_len > *out_size) {
1430+
free(rle);
1431+
free(packed);
1432+
*out_size = 0;
1433+
return NULL;
1434+
}
1435+
14071436
*out_size -= c_meta_len;
14081437
if (order && in_size < 8) {
14091438
out[0] &= ~1;
@@ -1413,8 +1442,20 @@ unsigned char *rans_compress_to_4x16(unsigned char *in, unsigned int in_size,
14131442
rans_enc_func(do_simd, order)(in, in_size, out+c_meta_len, out_size);
14141443

14151444
if (*out_size >= in_size) {
1445+
if (out > out_end) {
1446+
free(rle);
1447+
free(packed);
1448+
return NULL;
1449+
}
1450+
14161451
out[0] &= ~3;
14171452
out[0] |= RANS_ORDER_CAT | no_size;
1453+
1454+
if (out + c_meta_len + in_size > out_end) {
1455+
free(rle);
1456+
free(packed);
1457+
return NULL;
1458+
}
14181459
if (in_size)
14191460
memcpy(out+c_meta_len, in, in_size);
14201461
*out_size = in_size;

htscodecs/tokenise_name3.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1555,6 +1555,7 @@ uint8_t *tok3_encode_names(char *blk, int len, int level, int use_arith,
15551555
if (compress(ctx->desc[i].buf, ctx->desc[i].buf_l, i&0xf, level,
15561556
use_arith, out, &out_len) < 0) {
15571557
free_context(ctx);
1558+
free(out);
15581559
return NULL;
15591560
}
15601561

0 commit comments

Comments
 (0)