This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

• poc (icon.bmp, image/bmp, 2019-03-06 08:18:26 +0000, 63 bytes)

Reported in version: HG 1.2
Reported for operating system, platform: Linux, x86_64

Comments on the original bug report:

On 2019-03-06 08:18:26 +0000, zhangweiye(topsec) wrote:

Created attachment 3691
poc

I test this on ubuntu with SDL-19d8c3b9c251. Mv attach file to ./test/icon.bmp
then run ./testsprite.

=================================================================
==11110==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000009ad0 at pc 0x7f19a1fa5409 bp 0x7ffc91cf3df0 sp 0x7ffc91cf3de0
READ of size 2 at 0x602000009ad0 thread T0
    # 0 0x7f19a1fa5408 in BlitNtoN src/video/SDL_blit_N.c:2081
    # 1 0x7f19a1f63396 in SDL_SoftBlit src/video/SDL_blit.c:97
    # 2 0x7f19a1fc418a in SDL_LowerBlit src/video/SDL_surface.c:440
    # 3 0x7f19a1fc418a in SDL_ConvertSurface src/video/SDL_surface.c:866
    # 4 0x5570f1d2fe26 in LoadSprite /home/fuzz/Desktop/sdl/SDL-19d8c3b9c251/test/testsprite.c:49
    # 5 0x5570f1d2ef9a in main /home/fuzz/Desktop/sdl/SDL-19d8c3b9c251/test/testsprite.c:224
    # 6 0x7f19a1786b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)
    # 7 0x5570f1d2fc89 in _start (/home/fuzz/Desktop/sdl/SDL-19d8c3b9c251/test/testsprite+0x2c89)

0x602000009ad1 is located 0 bytes to the right of 1-byte region [0x602000009ad0,0x602000009ad1)
allocated by thread T0 here:
    # 0 0x7f19a23bfb50 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb50)
    # 1 0x7f19a1fbe031 in SDL_CreateRGBSurface src/video/SDL_surface.c:126

SUMMARY: AddressSanitizer: heap-buffer-overflow src/video/SDL_blit_N.c:2081 in BlitNtoN
Shadow bytes around the buggy address:
  0x0c047fff9300: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd
  0x0c047fff9310: fa fa fd fd fa fa fd fd fa fa fd fd fa fa 00 04
  0x0c047fff9320: fa fa 00 04 fa fa 00 04 fa fa 00 04 fa fa 00 04
  0x0c047fff9330: fa fa 00 04 fa fa 00 04 fa fa 00 04 fa fa 00 04
  0x0c047fff9340: fa fa 00 00 fa fa fd fd fa fa fd fd fa fa fd fd
=>0x0c047fff9350: fa fa fd fd fa fa fd fd fa fa[01]fa fa fa 00 00
  0x0c047fff9360: fa fa 00 00 fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9370: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9380: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9390: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff93a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==11110==ABORTING

On 2019-07-23 23:04:03 +0000, Ozkan Sezer wrote:

Both SDL-1.2 and SDL-2.0 hg versions of SDL_LoadBMP() succeed,
however SDL-2.0 fails in SDL_PixelFormatEnumToMasks() whereas
SDL-1.2 crashes.

This is a fuzz file, yes? SDL_LoadBMP() should have rejected
the file in the first place..

On 2019-07-24 19:38:46 +0000, Ozkan Sezer wrote:

This file gives biWidth = -1. So a quick fix would be something like:

diff --git a/src/video/SDL_bmp.c b/src/video/SDL_bmp.c
--- a/src/video/SDL_bmp.c
+++ b/src/video/SDL_bmp.c
@@ -143,6 +143,11 @@ SDL_Surface * SDL_LoadBMP_RW (SDL_RWops
(void) biYPelsPerMeter;
(void) biClrImportant;

• if (biWidth < 0) {

• SDL_SetError("BMP file with negative width");
  

• was_error = SDL_TRUE;
  

• goto done;
  

• }
  if (biHeight < 0) {
  topDown = SDL_TRUE;
  biHeight = -biHeight;

If this is acceptable, this should be applied to SDL2.0 too, and also
to SDL_image 1.2 and 2.0 versions.

Ryan, Sam?

On 2019-07-26 22:47:01 +0000, Ozkan Sezer wrote:

(In reply to Ozkan Sezer from comment # 2)

This file gives biWidth = -1. So a quick fix would be something like:

While we are there, we should probably reject zero width or height too.
Something like:

diff --git a/src/video/SDL_bmp.c b/src/video/SDL_bmp.c
--- a/src/video/SDL_bmp.c
+++ b/src/video/SDL_bmp.c
@@ -224,10 +224,15 @@ SDL_LoadBMP_RW(SDL_RWops * src, int free
headerSize = (Uint32) (SDL_RWtell(src) - (fp_offset + 14));
if (biSize > headerSize) {
SDL_RWseek(src, (biSize - headerSize), RW_SEEK_CUR);
}
}

• if (biWidth <= 0 || biHeight == 0) {

•    SDL_SetError("BMP file with bad dimensions (%dx%d)", biWidth, biHeight);
  

•    was_error = SDL_TRUE;
  

•    goto done;
  

• }
  if (biHeight < 0) {
  topDown = SDL_TRUE;
  biHeight = -biHeight;
  } else {
  topDown = SDL_FALSE;

Ryan, Sam: is this correct? OK to apply to SDL and SDL_image to both 1.2
and 2.0 branches?

On 2019-07-30 17:49:35 +0000, Ryan C. Gordon wrote:

(Sorry if you get several emails like this, we're marking a bunch of bugs.)

We're hoping to ship SDL 2.0.11 on a much shorter timeframe than we have historically done releases, so I'm starting to tag bugs we hope to have closed in this release cycle.

Note that this tag means we just intend to scrutinize this bug for the 2.0.11 release: we may fix it, reject it, or even push it back to a later release for now, but this helps give us both a goal and a wishlist for the next release.

If this bug has been quiet for a few months and you have new information (such as, "this is definitely still broken" or "this got fixed at some point"), please feel free to retest and/or add more notes to the bug.

--ryan.

On 2019-07-30 18:00:34 +0000, Sam Lantinga wrote:

Fixed, thanks!
https://hg.libsdl.org/SDL/rev/e7ba650a643a

On 2019-07-30 18:33:36 +0000, Ozkan Sezer wrote:

Fix applied to SDL-1.2 branches, too:
https://hg.libsdl.org/SDL/rev/ad1bbfbca760
https://hg.libsdl.org/SDL_image/rev/a59bfe382008

On 2019-08-02 08:52:59 +0000, Ozkan Sezer wrote:

This was assigned CVE-2019-13616

On 2019-09-20 20:47:34 +0000, Ryan C. Gordon wrote:

We're changing how we do SDL release versions; now releases will be even numbers (2.0.10, 2.0.12, etc), and as soon as we tag a release, we'll move the internal version number to an odd number (2.0.12 ships, we tag the latest in revision control as 2.0.13 immediately, which will become 2.0.14 on release, etc).

As such, I'm moving the bugs tagged with target-2.0.11 to target 2.0.12. Sorry if you get a lot of email from this change!

Thanks,
--ryan.

On 2019-09-20 20:48:42 +0000, Ryan C. Gordon wrote:

We're changing how we do SDL release versions; now releases will be even numbers (2.0.10, 2.0.12, etc), and as soon as we tag a release, we'll move the internal version number to an odd number (2.0.12 ships, we tag the latest in revision control as 2.0.13 immediately, which will become 2.0.14 on release, etc).

As such, I'm moving the bugs tagged with target-2.0.11 to target 2.0.12. Sorry if you get a lot of email from this change!

Thanks,
--ryan.