We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 4628

Summary: SEGV_UNKNOW in function SDL_free_REAL at SDL_malloc.c:5372-5
Product: SDL_image Reporter: pwd <teamseri0us360>
Component: miscAssignee: Sam Lantinga <slouken>
Status: RESOLVED FIXED QA Contact: Sam Lantinga <slouken>
Severity: normal    
Priority: P2 CC: hle
Version: 2.0.4   
Hardware: x86_64   
OS: Linux   
Attachments: poc
CVE-2019-12221: patch proposal (pcx: do not write directly to row buffer)

Description pwd 2019-05-09 06:49:35 UTC 
Created attachment 3780 [details]
poc

# libsdl2

## version

    libsdl2 2.0.9

## others

    please send email to  teamseri0us360@gmail.com if you have any questions.

---------------------

## SDL_free_REAL@SDL_malloc.c:5372-5___SEGV_UNKNOW

### description

    An issue was discovered in libsdl2 2.0.9 with SDL2_image-2.0.4, There is a SEGV_UNKNOW in function SDL_free_REAL at SDL_malloc.c:5372-5

### commandline

    loadjpg  @@ 

### source

```c
5368     if (!ptr) {
5369         return;
5370     }
5371 
>5372     s_mem.free_func(ptr);
5373     (void)SDL_AtomicDecRef(&s_mem.num_allocations);
5374 }
5375 
5376 /* vi: set ts=4 sw=4 expandtab: */

// Breakpoint 1, SDL_free_REAL (ptr=0x603ba0) at /src/libsdl2/src/stdlib/SDL_malloc.c:5372
// 5372        s_mem.free_func(ptr);
// (gdb) c
// Continuing.
// *** Error in `/src/loadjpg_g': munmap_chunk(): invalid pointer: 0x0000000000603ba0 ***
// ======= Backtrace: =========
// /lib/x86_64-linux-gnu/libc.so.6(+0x777e5)[0x7ffff75447e5]
// /lib/x86_64-linux-gnu/libc.so.6(cfree+0x1a8)[0x7ffff7551698]
// /src/libsdl2/installed-debug/lib/libSDL2-2.0.so.0(+0x5fa1c)[0x7ffff78f6a1c]
// /src/SDL2_image-2.0.4/installed-debug/lib/libSDL2_image-2.0.so.0(IMG_LoadPCX_RW+0x73f)[0x7ffff7bac8df]
// /src/SDL2_image-2.0.4/installed-debug/lib/libSDL2_image-2.0.so.0(IMG_LoadTyped_RW+0x150)[0x7ffff7ba8d00]
// /src/loadjpg2[0x4007e2]
// /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf0)[0x7ffff74ed830]
// /src/loadjpg2[0x4006d9]
// ======= Memory map: ========
// 00400000-00401000 r-xp 00000000 00:27 4750                               /src/loadjpg_g
// 00600000-00601000 r--p 00000000 00:27 4750                               /src/loadjpg_g
// 00601000-00602000 rw-p 00001000 00:27 4750                               /src/loadjpg_g
// ...
```

### bug report

```txt
ASAN:DEADLYSIGNAL
=================================================================
==7803==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x00000041c6ad bp 0x7ffc915be8c0 sp 0x7ffc915be020 T0)
    #0 0x41c6ac in __asan::asan_free(void*, __sanitizer::BufferedStackTrace*, __asan::AllocType) (/src/aflbuild/installed/bin/loadjpg+0x41c6ac)
    #1 0x4b89bc in __interceptor_cfree.localalias.0 (/src/aflbuild/installed/bin/loadjpg+0x4b89bc)
    #2 0x7f82c6095ae7 in SDL_free_REAL /src/libsdl2/src/stdlib/SDL_malloc.c:5372:5
    #3 0x7f82c61c7f01 in SDL_FreePalette_REAL /src/libsdl2/src/video/SDL_pixels.c:734:5
    #4 0x7f82c61c7f01 in SDL_SetPixelFormatPalette_REAL /src/libsdl2/src/video/SDL_pixels.c:685
    #5 0x7f82c61d9f96 in SDL_SetSurfacePalette_REAL /src/libsdl2/src/video/SDL_surface.c:221:9
    #6 0x7f82c61d9f96 in SDL_FreeSurface_REAL /src/libsdl2/src/video/SDL_surface.c:1233
    #7 0x7f82c652d0b1 in IMG_LoadPCX_RW /src/SDL2_image-2.0.4/IMG_pcx.c:252:13
    #8 0x7f82c651a9bd in IMG_LoadTyped_RW /src/SDL2_image-2.0.4/IMG.c:195:17
    #9 0x7f82c6519f41 in IMG_Load /src/SDL2_image-2.0.4/IMG.c:136:12
    #10 0x4ea140 in main /src/loadjpg.c:8:37
    #11 0x7f82c502082f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
    #12 0x418a38 in _start (/src/aflbuild/installed/bin/loadjpg+0x418a38)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (/src/aflbuild/installed/bin/loadjpg+0x41c6ac) in __asan::asan_free(void*, __sanitizer::BufferedStackTrace*, __asan::AllocType)
==7803==ABORTING

```
Comment 1 Hugo Lefeuvre 2019-05-25 11:32:37 UTC 
Created attachment 3797 [details]
CVE-2019-12221: patch proposal (pcx: do not write directly to row buffer)

Patch proposal in attachment.

I can provide more detailed explanations if needed.
Comment 2 Hugo Lefeuvre 2019-05-25 11:35:20 UTC 
(In reply to Hugo Lefeuvre from comment #1)
> Created attachment 3797 [details]
> CVE-2019-12221: patch proposal (pcx: do not write directly to row buffer)
> 
> Patch proposal in attachment.
> 
> I can provide more detailed explanations if needed.

(rationale is in the patch's header, putting it here as well for readability)

The PCX format specifies pcxh.BytesPerLine, which represents the size of a
single plane's scanline in bytes. Valid PCX images should have
pcxh.BytesPerLine >= surface->pitch.

pcxh.BytesPerLine and surface->pitch can legitimately be different because
pcxh.BytesPerLine is padded to be a multiple of machine word length (where
file was created).

If src_bits == 8 we directly read a whole scanline from src to row. This is
a problem in the case where bpl > surface->pitch because row is too small.

This allows attacker to perform unlimited OOB write on the heap.

+ remove pointless check bpl > surface->pitch, this is a valid situation
+ make sure we always read into buf which is big enough
+ in the case where src_bits == 8: copy these bytes back to row afterwards
Comment 3 Sam Lantinga 2019-06-10 22:41:25 UTC 
Fixed, thanks!
https://hg.libsdl.org/SDL_image/rev/e7e9786a1a34