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

oob in SDL_InvalidateMap #149

Closed
SDLBugzilla opened this issue Feb 11, 2021 · 3 comments
Closed

oob in SDL_InvalidateMap #149

SDLBugzilla opened this issue Feb 11, 2021 · 3 comments

Comments

@SDLBugzilla
Copy link
Collaborator

SDLBugzilla commented Feb 11, 2021

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

Reported in version: unspecified
Reported for operating system, platform: Linux, x86_64

Comments on the original bug report:

On 2019-05-05 03:04:23 +0000, pwd wrote:

Created attachment 3775
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

 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

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

On 2019-05-28 14:23:35 +0000, Hugo Lefeuvre wrote:

Created attachment 3799
[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

On 2019-05-28 14:24:30 +0000, Hugo Lefeuvre wrote:

This is a bug in SDL_Image.

This issue was assigned CVE-2019-12222.

On 2019-05-28 17:58:41 +0000, Ryan C. Gordon wrote:

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

On 2019-05-28 20:02:14 +0000, Hugo Lefeuvre wrote:

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?

On 2019-06-10 23:21:21 +0000, Sam Lantinga wrote:

This is fixed, thanks!

On 2019-06-19 10:00:35 +0000, Castro B wrote:

May i know the solution?

Castro B,
https://sparpedia.ch

On 2019-06-19 13:50:08 +0000, Sam Lantinga wrote:

https://hg.libsdl.org/SDL_image/rev/e7e9786a1a34

@Ashish123jha

This comment was marked as spam.

@MichaelHiness

This comment was marked as spam.

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

4 participants
@Ashish123jha @SDLBugzilla @MichaelHiness and others