bmp fuzz.diff

commit c9d1d14e56d7ed3cb3d43053c06a86229dc9ca65
Author: Nico Weber <[email protected]>
Date:   Sat Jul 18 22:35:01 2020 -0400

    bmp loader does not validate data_offset, can use that for arbitrary read, with <canvas> can probably leak arbitrary user mem to js

diff --git a/Libraries/LibGfx/BMPLoader.cpp b/Libraries/LibGfx/BMPLoader.cpp
index a6c82ca15..91299749a 100644
--- a/Libraries/LibGfx/BMPLoader.cpp
+++ b/Libraries/LibGfx/BMPLoader.cpp
@@ -29,7 +29,7 @@
 #include <AK/MappedFile.h>
 #include <LibGfx/BMPLoader.h>
 
-#define BMP_DEBUG 0
+#define BMP_DEBUG 1
 
 #define IF_BMP_DEBUG(x) \
     if (BMP_DEBUG)      \
@@ -186,6 +186,14 @@ RefPtr<Gfx::Bitmap> load_bmp(const StringView& path)
     return bitmap;
 }
 
+RefPtr<Gfx::Bitmap> load_bmp_from_memory(const u8* data, size_t length)
+{
+    auto bitmap = load_bmp_impl(data, length);
+    if (bitmap)
+        bitmap->set_mmap_name(String::format("Gfx::Bitmap [%dx%d] - Decoded BMP: <memory>", bitmap->width(), bitmap->height()));
+    return bitmap;
+}
+
 const LogStream& operator<<(const LogStream& out, Endpoint<i32> ep)
 {
     return out << "(" << ep.x << ", " << ep.y << ", " << ep.z << ")";
@@ -342,7 +350,7 @@ void populate_dib_mask_info(BMPLoadingContext& context)
     if (!mask_shifts.is_empty() && !mask_sizes.is_empty())
         return;
 
-    ASSERT(mask_shifts.is_empty() && mask_sizes.is_empty());
+    ASSERT(mask_shifts.is_empty() && mask_sizes.is_empty()); // XXX ??
 
     mask_shifts.ensure_capacity(masks.size());
     mask_sizes.ensure_capacity(masks.size());
@@ -473,6 +481,11 @@ static bool decode_bmp_header(BMPLoadingContext& context)
     // Ingore reserved bytes
     streamer.drop_bytes(4);
     context.data_offset = streamer.read_u32();
+    // XXX
+    if (context.data_offset >= context.data_size) {
+        return false;
+    }
+
     context.state = BMPLoadingContext::State::HeaderDecoded;
 
     IF_BMP_DEBUG(dbg() << "BMP data size: " << context.data_size);
@@ -779,6 +792,22 @@ static bool decode_bmp_dib(BMPLoadingContext& context)
         error = true;
     }
 
+    switch (context.dib.info.compression) {
+    case Compression::RGB:
+    case Compression::RLE8:
+    case Compression::RLE4:
+    case Compression::BITFIELDS:
+    case Compression::RLE24:
+    case Compression::PNG:
+    case Compression::ALPHABITFIELDS:
+    case Compression::CMYK:
+    case Compression::CMYKRLE8:
+    case Compression::CMYKRLE4:
+        break;
+    default:
+        error = true;
+    }
+
     if (!error && !set_dib_bitmasks(context, streamer))
         error = true;
 
@@ -852,7 +881,8 @@ static bool uncompress_bmp_rle_data(BMPLoadingContext& context, ByteBuffer& buff
         return false;
     }
 
-    Streamer streamer(context.data + context.data_offset, context.data_size);
+    //Streamer streamer(context.data + context.data_offset, context.data_size);
+    Streamer streamer(context.data + context.data_offset, context.data_size - context.data_offset);
 
     auto compression = context.dib.info.compression;
 
@@ -905,10 +935,11 @@ static bool uncompress_bmp_rle_data(BMPLoadingContext& context, ByteBuffer& buff
                 row++;
             }
             auto index = get_buffer_index();
-            if (index >= buffer.size()) {
+            if (index + 3 >= buffer.size()) {
                 IF_BMP_DEBUG(dbg() << "BMP has badly-formatted RLE data");
                 return false;
             }
+//dbg() << "index " << index << ", size " << buffer.size();
             ((u32&)buffer[index]) = color;
             column++;
             return true;
@@ -1098,13 +1129,17 @@ static bool decode_bmp_pixel_data(BMPLoadingContext& context)
 
     const u32 width = abs(context.dib.core.width);
     const u32 height = abs(context.dib.core.height);
+    if (width == 0 || width > 1 << 16 || height == 0 || height > 1 << 16)
+      return false;
+
     context.bitmap = Bitmap::create_purgeable(format, { static_cast<int>(width), static_cast<int>(height) });
     if (!context.bitmap) {
         IF_BMP_DEBUG(dbg() << "BMP appears to have overly large dimensions");
         return false;
     }
 
-    auto buffer = ByteBuffer::wrap(context.data + context.data_offset, context.data_size);
+    // XXX this isn't guaranteed to be valid ... (fixed it, now it is)
+    auto buffer = ByteBuffer::wrap(context.data + context.data_offset, context.data_size - context.data_offset);
 
     if (context.dib.info.compression == Compression::RLE4 || context.dib.info.compression == Compression::RLE8
         || context.dib.info.compression == Compression::RLE24) {
@@ -1144,7 +1179,9 @@ static bool decode_bmp_pixel_data(BMPLoadingContext& context)
             case 4: {
                 if (!streamer.has_u8())
                     return false;
+dbg() << 1;
                 u8 byte = streamer.read_u8();
+dbg() << 2;
                 context.bitmap->scanline_u8(row)[column++] = (byte >> 4) & 0xf;
                 if (column < width)
                     context.bitmap->scanline_u8(row)[column++] = byte & 0xf;
@@ -1173,7 +1210,8 @@ static bool decode_bmp_pixel_data(BMPLoadingContext& context)
                 if (context.dib.info.masks.is_empty()) {
                     context.bitmap->scanline(row)[column++] = streamer.read_u32() | 0xff000000;
                 } else {
-                    context.bitmap->scanline(row)[column++] = int_to_scaled_rgb(context, streamer.read_u32());
+                    u32 t = int_to_scaled_rgb(context, streamer.read_u32());
+                    context.bitmap->scanline(row)[column++] = t;
                 }
                 break;
             }
diff --git a/Libraries/LibGfx/BMPLoader.h b/Libraries/LibGfx/BMPLoader.h
index 6493b196e..fd223a600 100644
--- a/Libraries/LibGfx/BMPLoader.h
+++ b/Libraries/LibGfx/BMPLoader.h
@@ -33,6 +33,7 @@
 namespace Gfx {
 
 RefPtr<Gfx::Bitmap> load_bmp(const StringView& path);
+RefPtr<Gfx::Bitmap> load_bmp_from_memory(const u8* data, size_t length);
 
 struct BMPLoadingContext;
 

diff --git a/Meta/Lagom/ReadMe.md b/Meta/Lagom/ReadMe.md
index 42cfed428..ee0817bdf 100644
--- a/Meta/Lagom/ReadMe.md
+++ b/Meta/Lagom/ReadMe.md
@@ -18,3 +18,17 @@ Lagom can be used to fuzz parts of SerenityOS's code base. This requires buildli
     ninja FuzzJs && Meta/Lagom/Fuzzers/FuzzJs
 
 clang emits different warnings than gcc, so you'll likely have to remove `-Werror` in CMakeLists.txt and Meta/Lagom/CMakeLIsts.txt.
+
+Some fuzzers work better if you give them a fuzz corpus:
+
+    Meta/Lagom/Fuzzers/FuzzImageDecoder  ~/Downloads/gif/
+
+Can use this to repro a found crash too:
+
+    Meta/Lagom/Fuzzers/FuzzImageDecoder  ./crash-effdc3366da6d5c9ddd472572ad81570e4f99f20
+
+Less logging with `-close_fd_mask=3` (but hides assertion messages; just `1` only closes stdout).
+
+Parallel stuff with `-jobs=24 -workers=24`
+
+XXX coverage



commit f79061cc1aab0fab8e0c71908bfe9529e462503f
Author: Nico Weber <[email protected]>
Date:   Sat Jul 18 22:58:54 2020 -0400

    bmp loader now surives a few minutes of fuzzing

diff --git a/Libraries/LibGfx/BMPLoader.cpp b/Libraries/LibGfx/BMPLoader.cpp
index 91299749a..c3011c1e4 100644
--- a/Libraries/LibGfx/BMPLoader.cpp
+++ b/Libraries/LibGfx/BMPLoader.cpp
@@ -29,7 +29,7 @@
 #include <AK/MappedFile.h>
 #include <LibGfx/BMPLoader.h>
 
-#define BMP_DEBUG 1
+#define BMP_DEBUG 0
 
 #define IF_BMP_DEBUG(x) \
     if (BMP_DEBUG)      \
@@ -441,7 +441,8 @@ static bool set_dib_bitmasks(BMPLoadingContext& context, Streamer& streamer)
             context.dib.info.masks.append(streamer.read_u32());
 
         populate_dib_mask_info(context);
-    } else if (type >= DIBType::V2 && compression == Compression::BITFIELDS) {
+    } else if (type >= DIBType::V2 && (compression == Compression::BITFIELDS || !context.dib.info.masks.is_empty())) {
+        // XXX not sure if the !is_empty() is right, but it's what the decoding code checks for 32-bit, and that needs stuff populated
         populate_dib_mask_info(context);
     }
 
@@ -655,6 +656,7 @@ static bool decode_bmp_v2_dib(BMPLoadingContext& context, Streamer& streamer)
     if (!decode_bmp_info_dib(context, streamer))
         return false;
 
+// XXX
     context.dib.info.masks.append(streamer.read_u32());
     context.dib.info.masks.append(streamer.read_u32());
     context.dib.info.masks.append(streamer.read_u32());
@@ -838,8 +840,10 @@ static bool decode_bmp_color_table(BMPLoadingContext& context)
         return true;
     }
 
-    auto bytes_per_color = context.dib_type == DIBType::Core ? 3 : 4;
+    auto bytes_per_color = context.dib_type == DIBType::Core ? 3u : 4u;
     u32 max_colors = 1 << context.dib.core.bpp;
+    if (context.data_offset < bmp_header_size + context.dib_size())
+        return false;
     auto size_of_color_table = context.data_offset - bmp_header_size - context.dib_size();
 
     if (context.dib_type <= DIBType::OSV2) {
@@ -851,8 +855,9 @@ static bool decode_bmp_color_table(BMPLoadingContext& context)
         }
     }
 
+// XXX validate size_of_color_table
     Streamer streamer(context.data + bmp_header_size + context.dib_size(), size_of_color_table);
-    for (u32 i = 0; !streamer.at_end() && i < max_colors; ++i) {
+    for (u32 i = 0; streamer.remaining() >= bytes_per_color && i < max_colors; ++i) {
         if (bytes_per_color == 4) {
             context.color_table.append(streamer.read_u32());
         } else {
@@ -1042,7 +1047,11 @@ static bool uncompress_bmp_rle_data(BMPLoadingContext& context, ByteBuffer& buff
             if (byte == 1)
                 return true;
             if (byte == 2) {
+                if (!streamer.has_u8())
+                    return false;
                 u8 offset_x = streamer.read_u8();
+                if (!streamer.has_u8())
+                    return false;
                 u8 offset_y = streamer.read_u8();
                 column += offset_x;
                 if (column >= total_columns) {
@@ -1073,10 +1082,14 @@ static bool uncompress_bmp_rle_data(BMPLoadingContext& context, ByteBuffer& buff
             // Optionally consume a padding byte
             if (compression != Compression::RLE4) {
                 if (pixel_count % 2) {
+                    if (!streamer.has_u8())
+                        return false;
                     byte = streamer.read_u8();
                 }
             } else {
                 if (((pixel_count + 1) / 2) % 2) {
+                    if (!streamer.has_u8())
+                        return false;
                     byte = streamer.read_u8();
                 }
             }
@@ -1179,9 +1192,7 @@ static bool decode_bmp_pixel_data(BMPLoadingContext& context)
             case 4: {
                 if (!streamer.has_u8())
                     return false;
-dbg() << 1;
                 u8 byte = streamer.read_u8();
-dbg() << 2;
                 context.bitmap->scanline_u8(row)[column++] = (byte >> 4) & 0xf;
                 if (column < width)
                     context.bitmap->scanline_u8(row)[column++] = byte & 0xf;
@@ -1207,7 +1218,7 @@ dbg() << 2;
             case 32:
                 if (!streamer.has_u32())
                     return false;
-                if (context.dib.info.masks.is_empty()) {
+                if (context.dib.info.masks.is_empty()) {  // XXX 
                     context.bitmap->scanline(row)[column++] = streamer.read_u32() | 0xff000000;
                 } else {
                     u32 t = int_to_scaled_rgb(context, streamer.read_u32());