From ac865676a90c1776abb36b6f0d836b2489c8da7d Mon Sep 17 00:00:00 2001 From: Arman Hasanzadeh Date: Fri, 15 Aug 2025 12:32:50 -0700 Subject: [PATCH] Adds fbounds annotations for `VP8LColorCache`. Reasoning: Analysis of `VP8LColorCacheInit` (src/utils/color_cache_utils.c:29) revealed that `colors` is allocated using `WebPSafeCalloc` with a size of `1 << hash_bits`. Therefore, `colors` was annotated with `WEBP_COUNTED_BY_OR_NULL(1u << hash_bits)`. To support this, `WebPSafeCalloc` (src/utils/utils.h:59, src/utils/utils.c:214) was annotated to return `WEBP_SIZED_BY_OR_NULL(nmemb * size)`. Since `WebPSafeCalloc` returns a local pointer that defaults to unsafe when bounds safety is suppressed, `WEBP_UNSAFE_FORGE_BIDI_INDEXABLE` was used on the return value (src/utils/utils.c:222). Similarly, `VP8LColorCacheInit` required `WEBP_UNSAFE_FORGE_BIDI_INDEXABLE` when assigning the allocated pointer to the struct field (src/utils/color_cache_utils.c:47). Finally, `VP8LColorCacheInit` and `VP8LColorCacheClear` were modified to perform side-by-side assignments to `colors` and `hash_bits` as required by the `WEBP_COUNTED_BY_OR_NULL` annotation, using self-assignment for `hash_bits` when necessary to maintain functional equivalence with the original code. Bug: 432511821 Change-Id: I63cb46909d883a2e8932043ac3117b05b37e8d40 --- src/utils/color_cache_utils.c | 13 ++++++++++--- src/utils/color_cache_utils.h | 4 ++-- src/utils/utils.c | 5 +++-- src/utils/utils.h | 3 ++- 4 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/utils/color_cache_utils.c b/src/utils/color_cache_utils.c index 56f5353b..731a396d 100644 --- a/src/utils/color_cache_utils.c +++ b/src/utils/color_cache_utils.c @@ -28,13 +28,19 @@ WEBP_ASSUME_UNSAFE_INDEXABLE_ABI int VP8LColorCacheInit(VP8LColorCache* const color_cache, int hash_bits) { const int hash_size = 1 << hash_bits; + uint32_t* colors = (uint32_t*)WebPSafeCalloc((uint64_t)hash_size, + sizeof(*color_cache->colors)); assert(color_cache != NULL); assert(hash_bits > 0); - color_cache->colors = (uint32_t*)WebPSafeCalloc((uint64_t)hash_size, - sizeof(*color_cache->colors)); - if (color_cache->colors == NULL) return 0; + if (colors == NULL) { + color_cache->colors = NULL; + WEBP_SELF_ASSIGN(color_cache->hash_bits); + return 0; + } color_cache->hash_shift = 32 - hash_bits; color_cache->hash_bits = hash_bits; + color_cache->colors = WEBP_UNSAFE_FORGE_BIDI_INDEXABLE( + uint32_t*, colors, (size_t)hash_size * sizeof(*color_cache->colors)); return 1; } @@ -42,6 +48,7 @@ void VP8LColorCacheClear(VP8LColorCache* const color_cache) { if (color_cache != NULL) { WebPSafeFree(color_cache->colors); color_cache->colors = NULL; + WEBP_SELF_ASSIGN(color_cache->hash_bits); } } diff --git a/src/utils/color_cache_utils.h b/src/utils/color_cache_utils.h index d85203f6..0d21013c 100644 --- a/src/utils/color_cache_utils.h +++ b/src/utils/color_cache_utils.h @@ -30,8 +30,8 @@ extern "C" { // Main color cache struct. typedef struct { - uint32_t* colors; // color entries - int hash_shift; // Hash shift: 32 - 'hash_bits'. + uint32_t* WEBP_COUNTED_BY_OR_NULL(1u << hash_bits) colors; // color entries + int hash_shift; // Hash shift: 32 - 'hash_bits'. int hash_bits; } VP8LColorCache; diff --git a/src/utils/utils.c b/src/utils/utils.c index d0e15258..da2d9d79 100644 --- a/src/utils/utils.c +++ b/src/utils/utils.c @@ -211,14 +211,15 @@ void* WebPSafeMalloc(uint64_t nmemb, size_t size) { return ptr; } -void* WebPSafeCalloc(uint64_t nmemb, size_t size) { +void* WEBP_SIZED_BY_OR_NULL(nmemb* size) + WebPSafeCalloc(uint64_t nmemb, size_t size) { void* ptr; Increment(&num_calloc_calls); if (!CheckSizeArgumentsOverflow(nmemb, size)) return NULL; assert(nmemb * size > 0); ptr = calloc((size_t)nmemb, size); AddMem(ptr, (size_t)(nmemb * size)); - return ptr; + return WEBP_UNSAFE_FORGE_BIDI_INDEXABLE(void*, ptr, (size_t)(nmemb * size)); } void WebPSafeFree(void* const ptr) { diff --git a/src/utils/utils.h b/src/utils/utils.h index bcb9119c..8f440bab 100644 --- a/src/utils/utils.h +++ b/src/utils/utils.h @@ -56,7 +56,8 @@ static WEBP_INLINE int CheckSizeOverflow(uint64_t size) { WEBP_EXTERN void* WebPSafeMalloc(uint64_t nmemb, size_t size); // Note that WebPSafeCalloc() expects the second argument type to be 'size_t' // in order to favor the "calloc(num_foo, sizeof(foo))" pattern. -WEBP_EXTERN void* WebPSafeCalloc(uint64_t nmemb, size_t size); +WEBP_EXTERN void* WEBP_SIZED_BY_OR_NULL(nmemb* size) + WebPSafeCalloc(uint64_t nmemb, size_t size); // Companion deallocation function to the above allocations. WEBP_EXTERN void WebPSafeFree(void* const ptr);