potential integer overflow vulnerability in siril

hi,

seems there are multiple potential integer overflows in the code.

inside of import_pnm_to_fits() function from src/io/image_formats_internal.c,
fit->rx can be an arbitrary integer, which is input to the fits_flip_top_to_bottom().

if (fgets(buf, 256, file) == NULL) (* line 418 *)

fit->rx = atoi(buf); (* line 431 *)

fits_flip_top_to_bottom(fit); (* line 562 *)

inside of fits_flip_top_to_bottom() function from src/io/image_format_fits.c,
fit->rx is multiplied by sizeof(WORD), leading to a potential overflow, is used as an argument of malloc(), which might lead to a memory exploitation
line_size = fit->rx * sizeof(WORD);
swapline = malloc(line_size);

similar to 1), the tainted width value from user input flows through
debayer(src/algos/demosaicing.c) → debayer_buffer(src/algos/demosaicing.c) → bayer_VNG(src/algos/demosaicing.c)
and then finally used as an input to calloc() in bayer_VNG, where width is multiplied by 3, causing a potential integer overflow.

similar to 1), inside of import_pnm_to_fits() function from src/io/image_formats_internal.c,
fit->ry can be an arbitrary integer, which is multiplied by some number and then used an an argument of memory allocation functions as follows.

tmpbuf = malloc(stride * fit->ry); (* line 536 *)
fit->data = realloc(fit->data, stride * fit->ry * sizeof(WORD)); (* line 537 *)

inside of on_filechooser_file_set() from src/compositing/compositing.c,
the tainted user input(layers[layer]->the_fit) is input to copyfits() as below:

copyfits(&layers[layer]->the_fit, &gfit, CP_ALLOC | CP_FORMAT | CP_EXPAND, -1); (* line 505 *)

then, in copyfits() from src/io/image_format_fits.c, the tainted input is multiplied without any bound checking and used as argument of realloc() as below:

unsigned int nbdata = from->rx * from->ry; (* line 1349 *)
if (!(to->data = realloc(to->data, nbdata * depth * sizeof(WORD)))) { (* line 1378 *)

Thanks.

Hello.

Thanks a lot for your message.
However, which version of siril are you watching?
For example on master you don’t have fit->rx = atoi(buf); (* line 431 *) anymore. Now we use fit->rx = g_ascii_strtoull(buf, NULL, 10);

thanks for your reply,

I installed the distribution from ubuntu20.04,

which is v0.9.12.

OK. So even if some assertion are always true, the code has changed a lot since 0.9.12.
For Ubuntu we advice to install ppa repositories.

thanks!!

You’re welcome :).
And to make things easier, you could propose your patches to our gitlab too :).

We appreciate a lot your feedback.

Hi, how should we do a bounds check for image dimensions? We don’t want to limit ourselves to small images, and big allocations will fail anyway. How do you imagine these file readings to be protected?