Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
frokaikan opened this issue Aug 14, 2018 · 6 comments
Closed

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

frokaikan opened this issue Aug 14, 2018 · 6 comments

Comments

@frokaikan
Copy link

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
Copy link
Owner

wernsey commented Aug 14, 2018

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
Copy link
Author

frokaikan commented Aug 14, 2018

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
Copy link
Owner

wernsey commented Aug 14, 2018

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
Copy link
Owner

wernsey commented Aug 19, 2018

  • I've added support for loading 4-bit BMP files.
  • I've also made a note about the supported image formats in the README

@wernsey wernsey closed this as completed Aug 19, 2018
@abergmann
Copy link

CVE-2018-17073 was assigned to this issue.

@wernsey
Copy link
Owner

wernsey commented Sep 24, 2018

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

(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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants