[skcms] Harden CLUT indexing, enforce limits, and add test case - Add validation for grid point counts to enforce SKCMS_MAX_GRID_POINTS during parsing, protecting index calculations from integer overflow. - Secure clut() by clamping inputs to the [0, 1] range to prevent negative coordinate indexing and out-of-bounds (OOB) memory access. - Initialize index and weight arrays in clut() to avoid uninitialized memory use. - Add test case 'test_CLUT_OutOfBoundsInput' in tests.c to verify safe handling of extreme out-of-bounds input values on both B2A and A2B paths. Fixed: b/513702971 Change-Id: I3c07496be905ff553e9ee56b10eb586e5e839879 Reviewed-on: https://skia-review.googlesource.com/c/skcms/+/1241440 Commit-Queue: Kaylee Lubick <kjlubick@google.com> Reviewed-by: Kaylee Lubick <kjlubick@google.com>
diff --git a/skcms.cc b/skcms.cc index 889cef2..7117361 100644 --- a/skcms.cc +++ b/skcms.cc
@@ -35,6 +35,12 @@ using namespace skcms_private; +// A potential vulnerability exists where a large CLUT can cause an integer +// overflow in skcms's transformation logic. Limit the total number of grid +// points to a safe value. 350 million ensures that 6 * index will not overflow +// a 32-bit signed integer (which is what AVX2/AVX-512 gather expects). +#define SKCMS_MAX_GRID_POINTS 350000000 + static bool sAllowRuntimeCPUDetection = true; void skcms_DisableRuntimeCPUDetection() { @@ -757,11 +763,13 @@ return false; } + uint64_t total_grid_points = 1; for (uint32_t i = 0; i < a2b->input_channels; ++i) { a2b->grid_points[i] = mftTag->grid_points[0]; + total_grid_points *= a2b->grid_points[i]; } // The grid only makes sense with at least two points along each axis - if (a2b->grid_points[0] < 2) { + if (a2b->grid_points[0] < 2 || total_grid_points > SKCMS_MAX_GRID_POINTS) { return false; } return true; @@ -784,10 +792,12 @@ } // Same as A2B. + uint64_t total_grid_points = 1; for (uint32_t i = 0; i < b2a->input_channels; ++i) { b2a->grid_points[i] = mftTag->grid_points[0]; + total_grid_points *= b2a->grid_points[i]; } - if (b2a->grid_points[0] < 2) { + if (b2a->grid_points[0] < 2 || total_grid_points > SKCMS_MAX_GRID_POINTS) { return false; } return true; @@ -1036,6 +1046,7 @@ } uint64_t grid_size = a2b->output_channels * clut->grid_byte_width[0]; // the payload + uint64_t total_grid_points = 1; for (uint32_t i = 0; i < a2b->input_channels; ++i) { a2b->grid_points[i] = clut->grid_points[i]; // The grid only makes sense with at least two points along each axis @@ -1043,7 +1054,13 @@ return false; } grid_size *= a2b->grid_points[i]; + total_grid_points *= a2b->grid_points[i]; } + + if (total_grid_points > SKCMS_MAX_GRID_POINTS) { + return false; + } + const uint64_t table_size = clut_offset + SAFE_FIXED_SIZE(CLUT_Layout) + grid_size; if (table_size > tag->size) { return false; @@ -1175,13 +1192,20 @@ } uint64_t grid_size = b2a->output_channels * clut->grid_byte_width[0]; + uint64_t total_grid_points = 1; for (uint32_t i = 0; i < b2a->input_channels; ++i) { b2a->grid_points[i] = clut->grid_points[i]; if (b2a->grid_points[i] < 2) { return false; } grid_size *= b2a->grid_points[i]; + total_grid_points *= b2a->grid_points[i]; } + + if (total_grid_points > SKCMS_MAX_GRID_POINTS) { + return false; + } + if (tag->size < clut_offset + SAFE_FIXED_SIZE(CLUT_Layout) + grid_size) { return false; }
diff --git a/src/Transform_inl.h b/src/Transform_inl.h index 88d6c87..eb5c5ff 100644 --- a/src/Transform_inl.h +++ b/src/Transform_inl.h
@@ -680,19 +680,22 @@ F* r, F* g, F* b, F* a) { const int dim = (int)input_channels; - assert (0 < dim && dim <= 4); + if (dim <= 0 || dim > 4) { + return; + } assert (output_channels == 3 || output_channels == 4); // For each of these arrays, think foo[2*dim], but we use foo[8] since we know dim <= 4. - I32 index [8]; // Index contribution by dimension, first low from 0, then high from 4. - F weight[8]; // Weight for each contribution, again first low, then high. + I32 index [8] = {0,0,0,0, 0,0,0,0}; // Index contribution by dimension, first low from 0, then high from 4. + F weight[8] = {F0,F0,F0,F0, F0,F0,F0,F0}; // Weight for each contribution, again first low, then high. // O(dim) work first: calculate index,weight from r,g,b,a. const F inputs[] = { *r,*g,*b,*a }; for (int i = dim-1, stride = 1; i >= 0; i--) { // x is where we logically want to sample the grid in the i-th dimension. - F x = inputs[i] * (float)(grid_points[i] - 1); + // We MUST clamp to [0,1] here to avoid negative indices. + F x = max_(F0, min_(inputs[i], F1)) * (float)(grid_points[i] - 1); // But we can't index at floats. lo and hi are the two integer grid points surrounding x. I32 lo = cast<I32>( x ), // i.e. trunc(x) == floor(x) here.
diff --git a/tests.c b/tests.c index 52df42a..ed6383c 100644 --- a/tests.c +++ b/tests.c
@@ -2285,6 +2285,43 @@ #endif } +static void test_CLUT_OutOfBoundsInput(void) { + void* ptr; + size_t len; + expect(load_file("profiles/color.org/Upper_Left.icc", &ptr, &len)); + + skcms_ICCProfile profile; + expect(skcms_Parse(ptr, len, &profile)); + + // Create input float pixels containing negative, zero, normal, and very large values. + float src[] = { + -10.0f, -50.0f, -1.0f, 1.0f, + 0.0f, 0.5f, 1.0f, 1.0f, + 2.0f, 10.0f, 100.0f, 1.0f, + }; + uint8_t dst[12]; + + // This should run safely without crashing or out-of-bounds reads. + expect(skcms_Transform( + src, skcms_PixelFormat_RGBA_ffff, skcms_AlphaFormat_Unpremul, skcms_XYZD50_profile(), + dst, skcms_PixelFormat_RGBA_8888, skcms_AlphaFormat_Unpremul, &profile, + 3)); + + // Test A2B path as well with out-of-bounds float inputs. + float src_a2b[] = { + -10.0f, -50.0f, -1.0f, 1.0f, + 0.0f, 0.5f, 1.0f, 1.0f, + 2.0f, 10.0f, 100.0f, 1.0f, + }; + float dst_a2b[12]; + expect(skcms_Transform( + src_a2b, skcms_PixelFormat_RGBA_ffff, skcms_AlphaFormat_Unpremul, &profile, + dst_a2b, skcms_PixelFormat_RGBA_ffff, skcms_AlphaFormat_Unpremul, skcms_XYZD50_profile(), + 3)); + + free(ptr); +} + static void test_B2A(void) { void* ptr; size_t len; @@ -2387,6 +2424,7 @@ test_PQ_HLG_SRGB_xform(); test_RGBA_8888_sRGB(); test_ParseWithA2BPriority(); + test_CLUT_OutOfBoundsInput(); test_B2A(); test_CLUT_PageBoundary(); test_CLUT_PageBoundary2();