search

NOAA-EMC / NCEPLIBS-g2c

This library contains C decoder/encoder routines for GRIB edition 2.
Other
17 stars 11 forks source link

Jasper JPEG decoding issue - memory buffer too small #419

Closed EricEngle-NOAA closed 9 months ago

EricEngle-NOAA commented 1 year ago

When decoding a JPEG-encoded GRIB2 message, I am getting the following logging information from the Jasper library

maximum memory limit (10000000) would be exceeded
jas_image_decode: decode operation failed

This is occurring with some fairly large grids CMC Global (1500 x 721) and BOM Australia Global (2048 x 1536).

I see that in dec_jpeg2000.c, the memory size in is set here

https://github.com/NOAA-EMC/NCEPLIBS-g2c/blame/2a642e221587a13701cc01ed3e9c107299240f57/src/dec_jpeg2000.c#L61

Increasing the value does allow for a successful decode. I set it to 64MB for BOM Australia GRIB2 files, but I think it does not have to be that high and 32MB took care of the CMC GRIB2 messages.

edwardhartnett commented 1 year ago

Good catch!

What value should we use?

EricEngle-NOAA commented 1 year ago

The jasper library defines a default value, JAS_DEFAULT_MAX_MEM_USAGE, set to 1,073,741,824 bytes (1GB) in the CMake configuration. I think that is overkill for the needs of g2c. 256MB is probably a good number.

FWIW, if you comment out the call to jas_conf_set_max_mem_usage, you get the following warning:

warning: The application program did not set the memory limit for the JasPer library.
warning: The JasPer memory limit is being defaulted to a value that may be inappropriate for the system.  If the default is too small, some reasonable encoding/decoding operations will fail.  If the default is too large, security vulnerabilities will result (e.g., decoding a malicious image could exhaust all memory and crash the system.

On my Fedora VM, it set the value to 8.3GB -- half the system memory.

edwardhartnett commented 1 year ago

Can you add a CMake option to allow control of this? Then, if we select a number that's too big or small, at least the user can override it.

EricEngle-NOAA commented 1 year ago

That sounds good! I will leave the default at 256MB.

edwardhartnett commented 9 months ago

Should this issue be closed now?

EricEngle-NOAA commented 9 months ago

I think so. I think the solution in place is flexible enough and the default value is large enough.