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 4621

Summary: oob in SDL_InvalidateMap
Product: SDL_image Reporter: pwd <teamseri0us360>
Component: miscAssignee: Sam Lantinga <slouken>
Status: RESOLVED FIXED QA Contact: Sam Lantinga <slouken>
Severity: normal    
Priority: P2 CC: castro8583bennett, fieldengineer59, hle, icculus
Version: unspecified   
Hardware: x86_64   
OS: Linux   
URL: https://www.fieldengineer.com/blogs/how-field-engineer-helps-california-businesses-cut-costs-by-60
Attachments: poc
[PATCH] pcx: cast size and check calloc return value

Description pwd 2019-05-05 03:04:23 UTC 
Created attachment 3775 [details]
poc

## SDL_InvalidateMap@SDL_pixels.c:979-13___SEGV_UNKNOW

### description

    An issue was discovered in libsdl2 2.0.9 with SDL2_image-2.0.4, There is a/an SEGV_UNKNOW in function SDL_InvalidateMap at SDL_pixels.c:979-13

### commandline

    loadtif  @@ 

### source

```c
 975         return;
 976     }
 977     if (map->dst) {
 978         /* Release our reference to the surface - see the note below */
> 979         if (--map->dst->refcount <= 0) {
 980             SDL_FreeSurface(map->dst);
 981         }
 982     }
 983     map->dst = NULL;
 984     map->src_palette_version = 0;

// loadtif.c
// #include <stdio.h>
// #include <SDL.h>
// #include <SDL_image.h>
//
// int main(int argc, char * argv[]){
//         IMG_Init(IMG_INIT_TIF);//IMG_INIT_JPG);IMG_INIT_PNG
//         while(__AFL_LOOP(1000)){
//               SDL_Surface * image = IMG_Load(argv[1]);
//               if (image){
//                 SDL_FreeSurface(image);
//               }
//         }
//         IMG_Quit();
// }
```

### bug report

```txt
ASAN:DEADLYSIGNAL
=================================================================
==14609==ERROR: AddressSanitizer: SEGV on unknown address 0x1b60013f810b (pc 0x7f7210f5a77a bp 0x7ffc84c8bd80 sp 0x7ffc84c8ba80 T0)
    #0 0x7f7210f5a779 in SDL_InvalidateMap /src/libsdl2/src/video/SDL_pixels.c:979:13
    #1 0x7f7210f68c3a in SDL_FreeSurface_REAL /src/libsdl2/src/video/SDL_surface.c:1221:5
    #2 0x7f72112bc0b1 in IMG_LoadPCX_RW /src/SDL2_image-2.0.4/IMG_pcx.c:252:13
    #3 0x7f72112a99bd in IMG_LoadTyped_RW /src/SDL2_image-2.0.4/IMG.c:195:17
    #4 0x7f72112a8f41 in IMG_Load /src/SDL2_image-2.0.4/IMG.c:136:12
    #5 0x4ea0f0 in main /src/loadtif.c:8:37
    #6 0x7f720fdaf82f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
    #7 0x4189e8 in _start (/src/aflbuild/installed/bin/loadtif+0x4189e8)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /src/libsdl2/src/video/SDL_pixels.c:979:13 in SDL_InvalidateMap
==14609==ABORTING

```

### others

    from fuzz project pwd-libsdl2-loadtif-00
    crash name pwd-libsdl2-loadtif-00-00000007-20190419.tif
    Auto-generated by pyspider at 2019-04-19 00:37:05
Comment 1 Hugo Lefeuvre 2019-05-28 14:23:35 UTC 
Created attachment 3799 [details]
[PATCH] pcx: cast size and check calloc return value

Patch in attachment.

summary:

bpl is stored as a signed integer. If it happens to be negative, calloc
will be called with surface->pitch (since bpl < surface->pitch). Later we
call SDL_RWread(src, buf, bpl, 1). bpl is thus cast to size_t (becoming a
very large positive value), leading to obvious oob write.

We should fail early in this case. It doesn't make sense to continue
processing such files with corrupted bpl.

+ (size_t) cast bpl in SDL_max so that it is preferred over surface->pitch
if it is negative
+ check calloc return value to catch allocation failures
+ make sure we don't free unallocated buf in done section
Comment 2 Hugo Lefeuvre 2019-05-28 14:24:30 UTC 
This is a bug in SDL_Image.

This issue was assigned CVE-2019-12222.
Comment 3 Ryan C. Gordon 2019-05-28 17:58:41 UTC 
(In reply to Hugo Lefeuvre from comment #1)
> + make sure we don't free unallocated buf in done section

Minor nitpick:

+    if (buf) {
+        SDL_free(buf);
+    }

There's no need to check if buf != NULL here; SDL_free correctly ignores NULLs.

Other than that, this patch looks good to me.

--ryan.
Comment 4 Hugo Lefeuvre 2019-05-28 20:02:14 UTC 
> Minor nitpick:
> 
> +    if (buf) {
> +        SDL_free(buf);
> +    }
> 
> There's no need to check if buf != NULL here; SDL_free correctly ignores
> NULLs.
> 
> Other than that, this patch looks good to me.

Thanks for the review. Should I submit an updated version?
Comment 5 Sam Lantinga 2019-06-10 23:21:21 UTC 
This is fixed, thanks!
Comment 6 Castro B 2019-06-19 10:00:35 UTC 
May i know the solution?

Castro B,
https://sparpedia.ch
Comment 7 Sam Lantinga 2019-06-19 13:50:08 UTC 
https://hg.libsdl.org/SDL_image/rev/e7e9786a1a34
Comment 8 amez365 2021-02-09 12:31:45 UTC 
how does the gig economy work

Considered a haven for tech companies, entrepreneurs and savvy investors, California is home to numerous Fortune 500 companies. What’s more – it’s the most popular state for companies to base their headquarters. Currently, the headquarters of 20% of public companies in the US are in California, and it’s a trend which doesn’t show any signs of slowing down.

https://www.fieldengineer.com/blogs/how-field-engineer-helps-california-businesses-cut-costs-by-60