| Summary: | Heap-Buffer Overflow in Blit1to4 pertaining to SDL_blit_1.c | ||
|---|---|---|---|
| Product: | SDL | Reporter: | Radue <epitectus.agamemon> |
| Component: | video | Assignee: | Sam Lantinga <slouken> |
| Status: | RESOLVED FIXED | QA Contact: | Sam Lantinga <slouken> |
| Severity: | critical | ||
| Priority: | P2 | CC: | ameya.v.singh, ppisar |
| Version: | HG 2.1 | Keywords: | target-2.0.10 |
| Hardware: | x86_64 | ||
| OS: | Linux | ||
| Attachments: |
PoC
Fix for 1-, 4-, 8-bpp images Reject 2, 3, 5, 6, 7-bpp BMP images |
||
Assigned CVE-2019-7635 by MITRE. The root cause is that the POC BMP file declares 3 colors used and 4 bpp palette, but pixel at line 28 and column 1 (counted from 0) has color number 3. Then when the image loaded into a surface is passed to SDL_DisplayFormat(), in order to convert it to a video format, a used bliting function looks up a color number 3 in a 3-element long color bliting map. (The map obviously has the same number entries as the surface format has colors.) Proper fix should refuse broken BMP images that have a pixel with a color index higher than declared number of "used" colors. Possibly more advanced fix could try to relocate the out-of-range color index into a vacant index (if such exists). Created attachment 3637 [details]
Fix for 1-, 4-, 8-bpp images
By the way the BMP decoder assumes a less than 8 bit depth images have 1 or 4 bits per pixel. No other depths are correctly translated to an 8bpp surface. However, BMP specification does not forbid other depths like 2 or 3 bits per pixel. Based on reading the code, I believe pretty strange things can happen when displaying these images. My fix only deals with 1-, 4- and 8-bpp images because the decoder does not provide an easy access to pixel values for other bpp images. I believe it should be safer to reject the other-bpp completely. Created attachment 3645 [details]
Reject 2, 3, 5, 6, 7-bpp BMP images
This implements the rejection for strange-bpp images.
This is fixed, thanks! SDL 2.0: https://hg.libsdl.org/SDL/rev/7c643f1c1887 SDL 1.2: https://hg.libsdl.org/SDL/rev/08f3b4992538 https://hg.libsdl.org/SDL/rev/4646533663ae SDL_image: https://hg.libsdl.org/SDL_image/rev/03bd33e8cb49 > SDL 1.2:
> https://hg.libsdl.org/SDL/rev/08f3b4992538
> https://hg.libsdl.org/SDL/rev/4646533663ae
Maybe I don't understand hg enough, but it seems to me that these two commits are identical and contain the "Reject 2, 3, 5, 6, 7-bpp BMP images" fix.
SDL-1.2 branch is still missing the "Fix for 1-, 4-, 8-bpp images" fix.
Compilation failure due to duplication of switch case values. As per the line: https://github.com/spurious/SDL-mirror/blob/a88f1d07249cbe05e4d3fbd5e3b7b7269e4580c1/src/video/SDL_bmp.c#L40 The macro BI_RGB has not been defined any where in the source code. The line define BI_BITFIELDS as 3: https://github.com/spurious/SDL-mirror/blob/a88f1d07249cbe05e4d3fbd5e3b7b7269e4580c1/src/video/SDL_bmp.c#L44 Due to recent fixes in the following file reference: https://github.com/spurious/SDL-mirror/blob/a88f1d07249cbe05e4d3fbd5e3b7b7269e4580c1/src/video/SDL_bmp.c#L212 a switch case statement was added with the value 3, which results in compilation failure due to duplicate case value. Let me know in-case more information is needed. Regards, Ameya **Note:** Sorry for the external reference, since in my current development environment I do not have access to mercurial. >> SDL 1.2: >> https://hg.libsdl.org/SDL/rev/08f3b4992538 >> https://hg.libsdl.org/SDL/rev/4646533663ae > > Maybe I don't understand hg enough, but it seems to me that these two > commits are identical and contain the "Reject 2, 3, 5, 6, 7-bpp BMP images" > fix. [...] > Compilation failure due to duplication of switch case values. The two commits are not identical in the end. 08f3b4992538 is the correct fix identical to here proposed "Reject 2, 3, 5, 6, 7-bpp BMP images". 4646533663ae is a patch unknown to me that breaks the compilation. The 4646533663ae commit should be reverted. It does not make sense at all. Tagging a bunch of bugs with "target-2.0.10" so we have a clear list of things to address before a 2.0.10 release. Please note that "addressing" one of these bugs might mean deciding to defer on it until after 2.0.10, or resolving it as WONTFIX, etc. This is just here to tell us we should look at it carefully, and soon. If you have new information or feedback on this issue, this is a good time to add it to the conversation, as we're likely to be paying attention to this specific report in the next few days/weeks. Thanks! --ryan. I'm not sure why that second commit was added, sorry about that! It was fixed here: https://hg.libsdl.org/SDL/rev/33940ce0a0ba The "Fix for 1-, 4-, 8-bpp images" patch is still missing from SDL-1.2 branch. Patch added for SDL 1.2, thanks! https://hg.libsdl.org/SDL/rev/f1f5878be5db |
Created attachment 3603 [details] PoC A heap-buffer overflow vulnerability was discovered in SDL-1.2.15 library. Asan report: ================================================================= ==15035==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200006b49c at pc 0x7f2dc3d9a796 bp 0x7ffc133ade60 sp 0x7ffc133ade58 READ of size 4 at 0x60200006b49c thread T0 #0 0x7f2dc3d9a795 in Blit1to4 /home/radu/apps/sdl_player_lib/SDL-1.2.15/build/../src/video/SDL_blit_1.c:252:3 #1 0x7f2dc3d879ad in SDL_SoftBlit /home/radu/apps/sdl_player_lib/SDL-1.2.15/build/../src/video/SDL_blit.c:97:3 #2 0x7f2dc3df50fc in SDL_LowerBlit /home/radu/apps/sdl_player_lib/SDL-1.2.15/build/../src/video/SDL_surface.c:440:9 #3 0x7f2dc3df50fc in SDL_ConvertSurface /home/radu/apps/sdl_player_lib/SDL-1.2.15/build/../src/video/SDL_surface.c:866 #4 0x7f2dc3dfaa40 in SDL_DisplayFormat /home/radu/apps/sdl_player_lib/SDL-1.2.15/build/../src/video/SDL_video.c:946:9 #5 0x4dba11 in LoadSprite /home/radu/apps/sdl_player_lib/SDL-1.2.15/test/testsprite.c:41:9 #6 0x4dbc8f in main /home/radu/apps/sdl_player_lib/SDL-1.2.15/test/testsprite.c:98:7 #7 0x7f2dc2ace82f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291 #8 0x435528 in _start (/home/radu/apps/sdl_player_lib/SDL-1.2.15/test/testsprite+0x435528) 0x60200006b49c is located 0 bytes to the right of 12-byte region [0x60200006b490,0x60200006b49c) allocated by thread T0 here: #0 0x4bc4f2 in malloc (/home/radu/apps/sdl_player_lib/SDL-1.2.15/test/testsprite+0x4bc4f2) #1 0x7f2dc3de8195 in Map1toN /home/radu/apps/sdl_player_lib/SDL-1.2.15/build/../src/video/SDL_pixels.c:473:17 #2 0x7f2dc3de8195 in SDL_MapSurface /home/radu/apps/sdl_player_lib/SDL-1.2.15/build/../src/video/SDL_pixels.c:583 #3 0x7f2dc3df4deb in SDL_LowerBlit /home/radu/apps/sdl_player_lib/SDL-1.2.15/build/../src/video/SDL_surface.c:417:8 #4 0x7f2dc3df4deb in SDL_ConvertSurface /home/radu/apps/sdl_player_lib/SDL-1.2.15/build/../src/video/SDL_surface.c:866 #5 0x7f2dc3dfaa40 in SDL_DisplayFormat /home/radu/apps/sdl_player_lib/SDL-1.2.15/build/../src/video/SDL_video.c:946:9 SUMMARY: AddressSanitizer: heap-buffer-overflow /home/radu/apps/sdl_player_lib/SDL-1.2.15/build/../src/video/SDL_blit_1.c:252 Blit1to4 Shadow bytes around the buggy address: 0x0c0480005640: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c0480005650: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c0480005660: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c0480005670: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c0480005680: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa =>0x0c0480005690: fa fa 00[04]fa fa 00 00 fa fa 00 00 fa fa 00 00 0x0c04800056a0: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd 0x0c04800056b0: fa fa 00 00 fa fa 00 04 fa fa 00 04 fa fa 00 04 0x0c04800056c0: fa fa 00 04 fa fa 00 04 fa fa 00 04 fa fa 00 04 0x0c04800056d0: fa fa 00 04 fa fa fd fd fa fa fd fd fa fa fd fd 0x0c04800056e0: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd 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 Heap right redzone: fb Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack partial redzone: f4 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 ==15035==ABORTING Poc: See attachment Reproducing steps: 1. Download SDL-1.2.15 library 2. ./configure with Asan enabled 3. ./make 4. sudo make install 5. cd examples 6. ./configure with Asan enabled 7. make 8. ./testsprite PoC