Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix multiple potential vulnerabilities and bugs #1168

Merged
merged 8 commits into from
Dec 7, 2018
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]));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect this doesn't compile... Line 135 should be removed as it is effectively overrideen by line 136. Similar instances below

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have fixed that by make a new pr.

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