Skip to content

Commit

Permalink
Improve checking of *out_size in rans4x16 & arith
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@jkbonfield
jkbonfield committed Sep 2, 2024
1 parent 5179428 commit 34e6ee0
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 7 deletions.
25 changes: 21 additions & 4 deletions htscodecs/arith_dynamic.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -733,7 +733,7 @@ unsigned char *arith_compress_to(unsigned char *in, unsigned int in_size,
unsigned int c_meta_len;
uint8_t *rle = NULL, *packed = NULL;

if (in_size > INT_MAX) {
if (in_size > INT_MAX || (out && *out_size == 0)) {
*out_size = 0;
return NULL;
}
Expand All @@ -751,6 +751,8 @@ unsigned char *arith_compress_to(unsigned char *in, unsigned int in_size,
if (order & X_CAT) {
out[0] = X_CAT;
c_meta_len = 1 + var_put_u32(&out[1], out_end, in_size);
if (c_meta_len + in_size > *out_size)
return NULL;
memcpy(out+c_meta_len, in, in_size);
*out_size = in_size+c_meta_len;
}
Expand All @@ -759,6 +761,9 @@ unsigned char *arith_compress_to(unsigned char *in, unsigned int in_size,
int N = (order>>8);
if (N == 0) N = 4; // default for compatibility with old tests

if (N > in_size)
N = in_size;

if (N > 255)
return NULL;

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

for (j = 1; j <= m[MIN(i,3)][0]; j++) {
if (out2 - out > *out_size)
continue; // an error, but caught in best_sz check later

olen2 = *out_size - (out2 - out);
//fprintf(stderr, "order=%d m=%d\n", order&3, m[MIN(i,4)][j]);
if ((order&3) == 0 && (m[MIN(i,3)][j]&1))
continue;

arith_compress_to(transposed+idx[i], part_len[i],
out2, &olen2, m[MIN(i,3)][j] | X_NOSZ);
if (best_sz > olen2) {
if (olen2 && best_sz > olen2) {
best_sz = olen2;
best_j = j;
}
}
// if (best_j == 0) // none desireable
// return NULL;

if (best_sz == INT_MAX) {
free(transposed);
return NULL;
}
if (best_j != j-1) {
olen2 = *out_size - (out2 - out);
arith_compress_to(transposed+idx[i], part_len[i],
Expand Down Expand Up @@ -964,6 +975,12 @@ unsigned char *arith_compress_to(unsigned char *in, unsigned int in_size,
if (*out_size >= in_size) {
out[0] &= ~(3|X_EXT); // no entropy encoding, but keep e.g. PACK
out[0] |= X_CAT | no_size;

if (out + c_meta_len + in_size > out_end) {
free(rle);
free(packed);
return NULL;
}
memcpy(out+c_meta_len, in, in_size);
*out_size = in_size;
}
Expand Down
47 changes: 44 additions & 3 deletions htscodecs/rANS_static4x16pr.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1164,7 +1164,7 @@ void rans_set_cpu(int opts) {
unsigned char *rans_compress_to_4x16(unsigned char *in, unsigned int in_size,
unsigned char *out,unsigned int *out_size,
int order) {
if (in_size > INT_MAX) {
if (in_size > INT_MAX || (out && *out_size == 0)) {
*out_size = 0;
return NULL;
}
Expand Down Expand Up @@ -1199,6 +1199,12 @@ unsigned char *rans_compress_to_4x16(unsigned char *in, unsigned int in_size,
int N = (order>>8) & 0xff;
if (N == 0) N = 4; // default for compatibility with old tests

if (N > in_size)
N = in_size;

if (N > 255)
return NULL;

unsigned char *transposed = malloc(in_size);
unsigned int part_len[256];
unsigned int idx[256];
Expand Down Expand Up @@ -1249,26 +1255,31 @@ unsigned char *rans_compress_to_4x16(unsigned char *in, unsigned int in_size,
out2_start = out2 = out+7+5*N; // shares a buffer with c_meta
for (i = 0; i < N; i++) {
// Brute force try all methods.
int j, m[] = {1,64,128,0}, best_j = 0, best_sz = in_size+10;
int j, m[] = {1,64,128,0}, best_j = 0, best_sz = INT_MAX;
for (j = 0; j < sizeof(m)/sizeof(*m); j++) {
if ((order & m[j]) != m[j])
continue;

// order-1 *only*; bit check above cannot elide order-0
if ((order & RANS_ORDER_STRIPE_NO0) && (m[j]&1) == 0)
continue;

if (out2 - out > *out_size)
continue; // an error, but caught in best_sz check later

olen2 = *out_size - (out2 - out);
rans_compress_to_4x16(transposed+idx[i], part_len[i],
out2, &olen2,
m[j] | RANS_ORDER_NOSZ
| (order&RANS_ORDER_X32));
if (best_sz > olen2) {
if (olen2 && best_sz > olen2) {
best_sz = olen2;
best_j = j;
if (j < sizeof(m)/sizeof(*m) && olen2 > out_best_len) {
unsigned char *tmp = realloc(out_best, olen2);
if (!tmp) {
free(out_free);
free(transposed);
return NULL;
}
out_best = tmp;
Expand All @@ -1279,6 +1290,14 @@ unsigned char *rans_compress_to_4x16(unsigned char *in, unsigned int in_size,
memcpy(out_best, out2, olen2);
}
}

if (best_sz == INT_MAX) {
free(out_best);
free(out_free);
free(transposed);
return NULL;
}

if (best_j < sizeof(m)/sizeof(*m)) {
// Copy the best compression to output buffer if not current
memcpy(out2, out_best, best_sz);
Expand All @@ -1301,6 +1320,9 @@ unsigned char *rans_compress_to_4x16(unsigned char *in, unsigned int in_size,
out[0] = RANS_ORDER_CAT;
c_meta_len = 1;
c_meta_len += var_put_u32(&out[1], out_end, in_size);

if (c_meta_len + in_size > *out_size)
return NULL;
if (in_size)
memcpy(out+c_meta_len, in, in_size);
*out_size = c_meta_len + in_size;
Expand Down Expand Up @@ -1404,6 +1426,13 @@ unsigned char *rans_compress_to_4x16(unsigned char *in, unsigned int in_size,
out[0] &= ~RANS_ORDER_RLE;
}

if (c_meta_len > *out_size) {
free(rle);
free(packed);
*out_size = 0;
return NULL;
}

*out_size -= c_meta_len;
if (order && in_size < 8) {
out[0] &= ~1;
Expand All @@ -1413,8 +1442,20 @@ unsigned char *rans_compress_to_4x16(unsigned char *in, unsigned int in_size,
rans_enc_func(do_simd, order)(in, in_size, out+c_meta_len, out_size);

if (*out_size >= in_size) {
if (out > out_end) {
free(rle);
free(packed);
return NULL;
}

out[0] &= ~3;
out[0] |= RANS_ORDER_CAT | no_size;

if (out + c_meta_len + in_size > out_end) {
free(rle);
free(packed);
return NULL;
}
if (in_size)
memcpy(out+c_meta_len, in, in_size);
*out_size = in_size;
Expand Down
1 change: 1 addition & 0 deletions htscodecs/tokenise_name3.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1555,6 +1555,7 @@ uint8_t *tok3_encode_names(char *blk, int len, int level, int use_arith,
if (compress(ctx->desc[i].buf, ctx->desc[i].buf_l, i&0xf, level,
use_arith, out, &out_len) < 0) {
free_context(ctx);
free(out);
return NULL;
}

Expand Down

0 comments on commit 34e6ee0

Please sign in to comment.