wernsey / bitmap

A C module for manipulating bitmap/raster graphics
MIT No Attribution
69 stars 12 forks source link

SEGV on unknown access while get a bmp's width #1

Closed frokaikan closed 6 years ago

frokaikan commented 6 years ago

System: Ubuntu 18.04 Compile use: clang++ with asan, libpng, libjpeg Here's my program:

#include <cstdio>
#include "bmp.h"

typedef unsigned int uint;

int main(int argc,char** argv)
{
    if (argc == 1)
    {
        std::printf("No Input.\n");
        return 0;
    }

    // Load File
    Bitmap *bmp = bm_load(argv[1]);
    std::printf("Load OK\n");

    // Test W&H
    int w = bm_width(bmp);
    int h = bm_height(bmp);
    std::printf("W&H OK\n");

    // Test
    bm_clip(bmp,w/4,h/4,3*w/4,3*h/4);
    bm_flip_vertical(bmp);
    bm_unclip(bmp);
    uint col = bm_get(bmp,w/8,h/8);
    bm_set(bmp,w/8,h/8,0xFFFFFFFF-col);
    std::printf("Test OK\n");

    // Save
    int sav = bm_save(bmp,"out.bmp");
    if (sav)
        std::printf("Save OK\n");
    else
    {
        std::printf("Save Crash!\n");
        throw 1;
        return 1;
    }

    // free
    bm_free(bmp);

    std::printf("Finish\n");
    return 0;
}

and here is my bmp: not_kitty.zip

AddressSanitizer:DEADLYSIGNAL

==18470==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x000000553844 bp 0x000000572df0 sp 0x7ffc6aced1f0 T0) ==18470==The signal is caused by a READ memory access. ==18470==Hint: address points to the zero page.

0 0x553843 in bm_width /opt/bitmap/bmp.c:4255:15

#1 0x517f7e in main /opt/bitmap/mytest.cpp:19:10
#2 0x7fd86af7fb96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
#3 0x41b669 in _start (/opt/bitmap/mytest+0x41b669)

AddressSanitizer can not provide additional info. SUMMARY: AddressSanitizer: SEGV /opt/bitmap/bmp.c:4255:15 in bm_width ==18470==ABORTING

wernsey commented 6 years ago

Hi, your bitmap file, not_kitty.bmp is a 4-bit image, but the bitmap module only supports 8-bit palletized or 24-bit BMP images.

So the call to bm_load() on line 15 returned NULL which is why you got the segmentation fault in line 19 when you called bm_width().

I've been meaning to add support for more types in the future, but I haven't gotten around to it yet. I will update the documentation to mention that it only supports those two types in the meantime.

Here is a test program:

#include <stdio.h>
#include "bmp.h"

int main(int argc, char *argv[]) {
    if(argc < 2) return 1;
    Bitmap *bmp = bm_load(argv[1]);
    if(!bmp) {
        fprintf(stderr, "error loading %s: %s\n", argv[1], bm_last_error);
        return 1;
    }
    bm_save(bmp, "out.bmp");
    bm_free(bmp);
    printf("success\n");
    return 0;
}

Its output: error loading not_kitty.bmp: unsupported BMP type

frokaikan commented 6 years ago

and how about this PNG file? It got almost the same error. (I use libpng to compile the library.) not_kitty.zip

Is it the same reason as you say?

wernsey commented 6 years ago

Mmm, this is interesting. Your image is an 8-bit PNG, and I've used libpng's mechanisms to convert those images internally when the image is being loaded, but when I looked at the code on GitHub now I couldn't find it.

I see some error handling code is also missing, so I think I haven't pushed all of my most recent changes to GitHub. I'll look into it this evening.

I'll also keep your files as examples of images that the library should be able to load.

wernsey commented 6 years ago
abergmann commented 6 years ago

CVE-2018-17073 was assigned to this issue.

wernsey commented 6 years ago

I've added a bunch of assert()s to bmp.c to address this, in commit 1c88bbd728da6bff8f4533bcfdb0dfef4ed8038b.

(I've done it through asserts for performance reasons; so that the release version don't need to check for NULL pointers on every single call to the API functions)