search

InternationalColorConsortium / DemoIccMAX

Demonstration Implementation for iccMAX color profiles
Other
118 stars 37 forks source link

Excessive memory consumption and bad performance for Google SKIA profiles/fuzz/curv_size_overflow.icc #35

Closed petervwyatt closed 4 years ago

petervwyatt commented 4 years ago

When running "iccDumpProfile" CLI utility on PC or Linux with the Google SKIA file "profiles/fuzz/curv_size_overflow.icc" and either or both -v or all options, RefIccMax consumes excessive memory (multiple GBs) and is exceptionally slow as a result. wxDumpProfile on PC is much faster and does not seem to suffer this (but I did not check the code). Without -v or all options "iccDumpProfile" runs very quickly like wxDumpProfile.

As the ICC profile filename suggests, the root cause is that the RGB TRC 'curv' have counts (n) of 2,147,484,348 which then attempts to allocate gigantic buffers * sizeof(float)! This is because the ICC file has been fuzzed and the corresponding bytes for count in the file are all 0xFF... (so, yes, it is malformed).

Problematic code is in IccProfLib/IccTagLut.cpp, function bool CIccTagCurve::SetSize(...) from line 442 (https://github.com/InternationalColorConsortium/RefIccMAX/blob/8da81e9151581161a582dbc4ca0ed8220aba02a9/IccProfLib/IccTagLut.cpp#L442). Would suggest that santity checks on the magnitude of count (n) prior to doing malloc/icRealloc are required. There may even be a maximum possible value for count given that the data for 'curv' are all uInt16Numbers according to ICC.1 v4.3.0 and the curves may have some precision requirements but all that color theory is beyond me...

Also suggest checking that all other malloc/icRealloc calls in the codebase are also range-checked prior to calls.

To get the Google SKIA ICC corpus and skcms/profiles/fuzz/curv_size_overflow.icc:

git clone https://skia.googlesource.com/skcms    ## quite small repo (~180MB), with corpus in 'profiles' folder and below
cd skcms/profiles/fuzz
maxderhak commented 4 years ago

Read functions have been improved to check sizes before allocating. Failure is now immediate for the curve_size_overflow.icc.