Browse Source

LibGfx: Fix dynamic bitmasks in BMPs

I overlooked a corner case where we might call the built-in ctz() on zero.

Furthermore, the calculation of the shift was wrong and the results were often
unusable.

Both issue were caused by a forgotten 36daeee34ff04f64c933e94a9cdffe9080061fb0.
This time I made sure to look at bmpsuite_files first, and now they look good.

Found by OSS-Fuzz:
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28985
Ben Wiederhake 4 years ago
parent
commit
4332dfb964
2 changed files with 16 additions and 3 deletions
  1. 8 1
      AK/Platform.h
  2. 8 2
      Userland/Libraries/LibGfx/BMPLoader.cpp

+ 8 - 1
AK/Platform.h

@@ -81,9 +81,16 @@ ALWAYS_INLINE int count_trailing_zeroes_32(unsigned int val)
     }
     return 0;
 #endif
+}
+
+ALWAYS_INLINE int count_trailing_zeroes_32_safe(unsigned int val)
+{
+    if (val == 0)
+        return 32;
+    return count_trailing_zeroes_32(val);
+}
 
 #ifdef AK_OS_BSD_GENERIC
 #    define CLOCK_MONOTONIC_COARSE CLOCK_MONOTONIC
 #    define CLOCK_REALTIME_COARSE CLOCK_REALTIME
 #endif
-}

+ 8 - 2
Userland/Libraries/LibGfx/BMPLoader.cpp

@@ -368,8 +368,14 @@ static void populate_dib_mask_info_if_needed(BMPLoadingContext& context)
             continue;
         }
         int trailing_zeros = count_trailing_zeroes_32(mask);
-        int size = count_trailing_zeroes_32(~(mask >> trailing_zeros));
-        mask_shifts.append(trailing_zeros - 8);
+        // If mask is exactly `0xFFFFFFFF`, then we might try to count the trailing zeros of 0x00000000 here, so we need the safe version:
+        int size = count_trailing_zeroes_32_safe(~(mask >> trailing_zeros));
+        if (size > 8) {
+            // Drop lowest bits if mask is longer than 8 bits.
+            trailing_zeros += size - 8;
+            size = 8;
+        }
+        mask_shifts.append(size + trailing_zeros - 8);
         mask_sizes.append(size);
     }
 }