Skip to content

Commit

Permalink
Merge pull request #1168 from Young-X/fix_dev
Browse files Browse the repository at this point in the history
Fix multiple potential vulnerabilities and bugs
  • Loading branch information
rouault authored Dec 7, 2018
2 parents e0f5212 + 05be308 commit e7640f5
Show file tree
Hide file tree
Showing 5 changed files with 132 additions and 27 deletions.
59 changes: 42 additions & 17 deletions src/bin/jpwl/convert.c
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
#include <stdlib.h>
#include <string.h>
#include <ctype.h>
#include <limits.h>

#ifdef OPJ_HAVE_LIBTIFF
#include <tiffio.h>
Expand Down Expand Up @@ -98,15 +99,10 @@ struct tga_header {
};
#endif /* INFORMATION_ONLY */

static unsigned short get_ushort(unsigned short val)
/* Returns a ushort from a little-endian serialized value */
static unsigned short get_tga_ushort(const unsigned char *data)
{

#ifdef OPJ_BIG_ENDIAN
return (((val & 0xff) << 8) + (val >> 8));
#else
return (val);
#endif

return data[0] | (data[1] << 8);
}

#define TGA_HEADER_SIZE 18
Expand Down Expand Up @@ -135,15 +131,15 @@ static int tga_readheader(FILE *fp, unsigned int *bits_per_pixel,
id_len = (unsigned char)tga[0];
cmap_type = (unsigned char)tga[1];
image_type = (unsigned char)tga[2];
cmap_index = get_ushort(*(unsigned short*)(&tga[3]));
cmap_len = get_ushort(*(unsigned short*)(&tga[5]));
cmap_index = get_tga_ushort(*(unsigned short*)(&tga[3]));
cmap_len = get_tga_ushort(*(unsigned short*)(&tga[5]));
cmap_entry_size = (unsigned char)tga[7];


x_origin = get_ushort(*(unsigned short*)(&tga[8]));
y_origin = get_ushort(*(unsigned short*)(&tga[10]));
image_w = get_ushort(*(unsigned short*)(&tga[12]));
image_h = get_ushort(*(unsigned short*)(&tga[14]));
x_origin = get_tga_ushort(*(unsigned short*)(&tga[8]));
y_origin = get_tga_ushort(*(unsigned short*)(&tga[10]));
image_w = get_tga_ushort(*(unsigned short*)(&tga[12]));
image_h = get_tga_ushort(*(unsigned short*)(&tga[14]));
pixel_depth = (unsigned char)tga[16];
image_desc = (unsigned char)tga[17];

Expand Down Expand Up @@ -333,6 +329,24 @@ opj_image_t* tgatoimage(const char *filename, opj_cparameters_t *parameters)
color_space = CLRSPC_SRGB;
}

/* If the declared file size is > 10 MB, check that the file is big */
/* enough to avoid excessive memory allocations */
if (image_height != 0 && image_width > 10000000 / image_height / numcomps) {
char ch;
OPJ_UINT64 expected_file_size =
(OPJ_UINT64)image_width * image_height * numcomps;
long curpos = ftell(f);
if (expected_file_size > (OPJ_UINT64)INT_MAX) {
expected_file_size = (OPJ_UINT64)INT_MAX;
}
fseek(f, (long)expected_file_size - 1, SEEK_SET);
if (fread(&ch, 1, 1, f) != 1) {
fclose(f);
return NULL;
}
fseek(f, curpos, SEEK_SET);
}

subsampling_dx = parameters->subsampling_dx;
subsampling_dy = parameters->subsampling_dy;

Expand Down Expand Up @@ -444,7 +458,7 @@ int imagetotga(opj_image_t * image, const char *outfile)
{
int width, height, bpp, x, y;
opj_bool write_alpha;
int i, adjustR, adjustG, adjustB;
int i, adjustR, adjustG = 0, adjustB = 0;
unsigned int alpha_channel;
float r, g, b, a;
unsigned char value;
Expand Down Expand Up @@ -485,8 +499,10 @@ int imagetotga(opj_image_t * image, const char *outfile)
scale = 255.0f / (float)((1 << image->comps[0].prec) - 1);

adjustR = (image->comps[0].sgnd ? 1 << (image->comps[0].prec - 1) : 0);
adjustG = (image->comps[1].sgnd ? 1 << (image->comps[1].prec - 1) : 0);
adjustB = (image->comps[2].sgnd ? 1 << (image->comps[2].prec - 1) : 0);
if (image->numcomps >= 3) {
adjustG = (image->comps[1].sgnd ? 1 << (image->comps[1].prec - 1) : 0);
adjustB = (image->comps[2].sgnd ? 1 << (image->comps[2].prec - 1) : 0);
}

for (y = 0; y < height; y++) {
unsigned int index = y * width;
Expand Down Expand Up @@ -1862,6 +1878,15 @@ opj_image_t* pnmtoimage(const char *filename, opj_cparameters_t *parameters)
return NULL;
}

/* This limitation could be removed by making sure to use size_t below */
if (header_info.height != 0 &&
header_info.width > INT_MAX / header_info.height) {
fprintf(stderr, "pnmtoimage:Image %dx%d too big!\n",
header_info.width, header_info.height);
fclose(fp);
return NULL;
}

format = header_info.format;

switch (format) {
Expand Down
5 changes: 3 additions & 2 deletions src/bin/jpwl/opj_jpwl_compress.c
Original file line number Diff line number Diff line change
Expand Up @@ -952,8 +952,9 @@ static int parse_cmdline_encoder(int argc, char **argv,
case 'b': { /* code-block dimension */
int cblockw_init = 0, cblockh_init = 0;
sscanf(opj_optarg, "%d,%d", &cblockw_init, &cblockh_init);
if (cblockw_init * cblockh_init > 4096 || cblockw_init > 1024
|| cblockw_init < 4 || cblockh_init > 1024 || cblockh_init < 4) {
if (cblockw_init > 1024 || cblockw_init < 4 ||
cblockh_init > 1024 || cblockh_init < 4 ||
cblockw_init * cblockh_init > 4096) {
fprintf(stderr,
"!! Size of code_block error (option -b) !!\n\nRestriction :\n"
" * width*height<=4096\n * 4<=width,height<= 1024\n\n");
Expand Down
19 changes: 11 additions & 8 deletions src/lib/openjp2/pi.c
Original file line number Diff line number Diff line change
Expand Up @@ -748,6 +748,9 @@ static void opj_get_encoding_parameters(const opj_image_t *p_image,
/* position in x and y of tile */
OPJ_UINT32 p, q;

/* non-corrected (in regard to image offset) tile offset */
OPJ_UINT32 l_tx0, l_ty0;

/* preconditions */
assert(p_cp != 00);
assert(p_image != 00);
Expand All @@ -763,14 +766,14 @@ static void opj_get_encoding_parameters(const opj_image_t *p_image,
q = p_tileno / p_cp->tw;

/* find extent of tile */
*p_tx0 = opj_int_max((OPJ_INT32)(p_cp->tx0 + p * p_cp->tdx),
(OPJ_INT32)p_image->x0);
*p_tx1 = opj_int_min((OPJ_INT32)(p_cp->tx0 + (p + 1) * p_cp->tdx),
(OPJ_INT32)p_image->x1);
*p_ty0 = opj_int_max((OPJ_INT32)(p_cp->ty0 + q * p_cp->tdy),
(OPJ_INT32)p_image->y0);
*p_ty1 = opj_int_min((OPJ_INT32)(p_cp->ty0 + (q + 1) * p_cp->tdy),
(OPJ_INT32)p_image->y1);
l_tx0 = p_cp->tx0 + p *
p_cp->tdx; /* can't be greater than p_image->x1 so won't overflow */
*p_tx0 = (OPJ_INT32)opj_uint_max(l_tx0, p_image->x0);
*p_tx1 = (OPJ_INT32)opj_uint_min(opj_uint_adds(l_tx0, p_cp->tdx), p_image->x1);
l_ty0 = p_cp->ty0 + q *
p_cp->tdy; /* can't be greater than p_image->y1 so won't overflow */
*p_ty0 = (OPJ_INT32)opj_uint_max(l_ty0, p_image->y0);
*p_ty1 = (OPJ_INT32)opj_uint_min(opj_uint_adds(l_ty0, p_cp->tdy), p_image->y1);

/* max precision is 0 (can only grow) */
*p_max_prec = 0;
Expand Down
24 changes: 24 additions & 0 deletions src/lib/openjp3d/pi.c
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,14 @@ static bool pi_next_rpcl(opj_pi_iterator_t * pi)
rpx = res->pdx + levelnox;
rpy = res->pdy + levelnoy;
rpz = res->pdz + levelnoz;

/* To avoid divisions by zero / undefined behaviour on shift */
if (rpx >= 31 || ((comp->dx << rpx) >> rpx) != comp->dx ||
rpy >= 31 || ((comp->dy << rpy) >> rpy) != comp->dy ||
rpz >= 31 || ((comp->dz << rpz) >> rpz) != comp->dz) {
continue;
}

if ((!(pi->x % (comp->dx << rpx) == 0) || (pi->x == pi->tx0 &&
(trx0 << levelnox) % (1 << rpx)))) {
continue;
Expand Down Expand Up @@ -329,6 +337,14 @@ static bool pi_next_pcrl(opj_pi_iterator_t * pi)
rpx = res->pdx + levelnox;
rpy = res->pdy + levelnoy;
rpz = res->pdz + levelnoz;

/* To avoid divisions by zero / undefined behaviour on shift */
if (rpx >= 31 || ((comp->dx << rpx) >> rpx) != comp->dx ||
rpy >= 31 || ((comp->dy << rpy) >> rpy) != comp->dy ||
rpz >= 31 || ((comp->dz << rpz) >> rpz) != comp->dz) {
continue;
}

if ((!(pi->x % (comp->dx << rpx) == 0) || (pi->x == pi->tx0 &&
(trx0 << levelnox) % (1 << rpx)))) {
continue;
Expand Down Expand Up @@ -432,6 +448,14 @@ static bool pi_next_cprl(opj_pi_iterator_t * pi)
rpx = res->pdx + levelnox;
rpy = res->pdy + levelnoy;
rpz = res->pdz + levelnoz;

/* To avoid divisions by zero / undefined behaviour on shift */
if (rpx >= 31 || ((comp->dx << rpx) >> rpx) != comp->dx ||
rpy >= 31 || ((comp->dy << rpy) >> rpy) != comp->dy ||
rpz >= 31 || ((comp->dz << rpz) >> rpz) != comp->dz) {
continue;
}

if ((!(pi->x % (comp->dx << rpx) == 0) || (pi->x == pi->tx0 &&
(trx0 << levelnox) % (1 << rpx)))) {
continue;
Expand Down
52 changes: 52 additions & 0 deletions src/lib/openmj2/pi.c
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,12 @@ static opj_bool pi_next_cprl(opj_pi_iterator_t * pi);
==========================================================
*/

static void opj_pi_emit_error(opj_pi_iterator_t * pi, const char* msg)
{
(void)pi;
(void)msg;
}

static opj_bool pi_next_lrcp(opj_pi_iterator_t * pi)
{
opj_pi_comp_t *comp = NULL;
Expand Down Expand Up @@ -114,6 +120,11 @@ static opj_bool pi_next_lrcp(opj_pi_iterator_t * pi)
for (pi->precno = pi->poc.precno0; pi->precno < pi->poc.precno1; pi->precno++) {
index = pi->layno * pi->step_l + pi->resno * pi->step_r + pi->compno *
pi->step_c + pi->precno * pi->step_p;
/* Avoids index out of bounds access with include*/
if (index >= pi->include_size) {
opj_pi_emit_error(pi, "Invalid access to pi->include");
return OPJ_FALSE;
}
if (!pi->include[index]) {
pi->include[index] = 1;
return OPJ_TRUE;
Expand Down Expand Up @@ -156,6 +167,11 @@ static opj_bool pi_next_rlcp(opj_pi_iterator_t * pi)
for (pi->precno = pi->poc.precno0; pi->precno < pi->poc.precno1; pi->precno++) {
index = pi->layno * pi->step_l + pi->resno * pi->step_r + pi->compno *
pi->step_c + pi->precno * pi->step_p;
/* Avoids index out of bounds access with include*/
if (index >= pi->include_size) {
opj_pi_emit_error(pi, "Invalid access to pi->include");
return OPJ_FALSE;
}
if (!pi->include[index]) {
pi->include[index] = 1;
return OPJ_TRUE;
Expand Down Expand Up @@ -224,6 +240,13 @@ static opj_bool pi_next_rpcl(opj_pi_iterator_t * pi)
try1 = int_ceildiv(pi->ty1, comp->dy << levelno);
rpx = res->pdx + levelno;
rpy = res->pdy + levelno;

/* To avoid divisions by zero / undefined behaviour on shift */
if (rpx >= 31 || ((comp->dx << rpx) >> rpx) != comp->dx ||
rpy >= 31 || ((comp->dy << rpy) >> rpy) != comp->dy) {
continue;
}

if (!((pi->y % (comp->dy << rpy) == 0) || ((pi->y == pi->ty0) &&
((try0 << levelno) % (1 << rpy))))) {
continue;
Expand All @@ -249,6 +272,11 @@ static opj_bool pi_next_rpcl(opj_pi_iterator_t * pi)
for (pi->layno = pi->poc.layno0; pi->layno < pi->poc.layno1; pi->layno++) {
index = pi->layno * pi->step_l + pi->resno * pi->step_r + pi->compno *
pi->step_c + pi->precno * pi->step_p;
/* Avoids index out of bounds access with include*/
if (index >= pi->include_size) {
opj_pi_emit_error(pi, "Invalid access to pi->include");
return OPJ_FALSE;
}
if (!pi->include[index]) {
pi->include[index] = 1;
return OPJ_TRUE;
Expand Down Expand Up @@ -317,6 +345,13 @@ static opj_bool pi_next_pcrl(opj_pi_iterator_t * pi)
try1 = int_ceildiv(pi->ty1, comp->dy << levelno);
rpx = res->pdx + levelno;
rpy = res->pdy + levelno;

/* To avoid divisions by zero / undefined behaviour on shift */
if (rpx >= 31 || ((comp->dx << rpx) >> rpx) != comp->dx ||
rpy >= 31 || ((comp->dy << rpy) >> rpy) != comp->dy) {
continue;
}

if (!((pi->y % (comp->dy << rpy) == 0) || ((pi->y == pi->ty0) &&
((try0 << levelno) % (1 << rpy))))) {
continue;
Expand All @@ -342,6 +377,11 @@ static opj_bool pi_next_pcrl(opj_pi_iterator_t * pi)
for (pi->layno = pi->poc.layno0; pi->layno < pi->poc.layno1; pi->layno++) {
index = pi->layno * pi->step_l + pi->resno * pi->step_r + pi->compno *
pi->step_c + pi->precno * pi->step_p;
/* Avoids index out of bounds access with include*/
if (index >= pi->include_size) {
opj_pi_emit_error(pi, "Invalid access to pi->include");
return OPJ_FALSE;
}
if (!pi->include[index]) {
pi->include[index] = 1;
return OPJ_TRUE;
Expand Down Expand Up @@ -408,6 +448,13 @@ static opj_bool pi_next_cprl(opj_pi_iterator_t * pi)
try1 = int_ceildiv(pi->ty1, comp->dy << levelno);
rpx = res->pdx + levelno;
rpy = res->pdy + levelno;

/* To avoid divisions by zero / undefined behaviour on shift */
if (rpx >= 31 || ((comp->dx << rpx) >> rpx) != comp->dx ||
rpy >= 31 || ((comp->dy << rpy) >> rpy) != comp->dy) {
continue;
}

if (!((pi->y % (comp->dy << rpy) == 0) || ((pi->y == pi->ty0) &&
((try0 << levelno) % (1 << rpy))))) {
continue;
Expand All @@ -433,6 +480,11 @@ static opj_bool pi_next_cprl(opj_pi_iterator_t * pi)
for (pi->layno = pi->poc.layno0; pi->layno < pi->poc.layno1; pi->layno++) {
index = pi->layno * pi->step_l + pi->resno * pi->step_r + pi->compno *
pi->step_c + pi->precno * pi->step_p;
/* Avoids index out of bounds access with include*/
if (index >= pi->include_size) {
opj_pi_emit_error(pi, "Invalid access to pi->include");
return OPJ_FALSE;
}
if (!pi->include[index]) {
pi->include[index] = 1;
return OPJ_TRUE;
Expand Down

0 comments on commit e7640f5

Please sign in to comment.