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 2965

Summary: Edit NULL check for src in load_xpm function
Product: SDL_image Reporter: Nitz <nitin.j4>
Component: miscAssignee: Ryan C. Gordon <icculus>
Status: RESOLVED FIXED QA Contact: Sam Lantinga <slouken>
Severity: major    
Priority: P2 CC: icculus, sylvain.becker
Version: unspecified   
Hardware: x86   
OS: Linux   
Attachments: Patch for src NULL check

Description Nitz 2015-04-29 05:13:57 UTC
Created attachment 2140 [details]
Patch for src NULL check

In function load_xpm:

    if (src)
        start = SDL_RWtell(src);

    if (xpm)
        xpmlines = &xpm;

    line = get_next_line(xpmlines, src, 0);


Here if src is NULL then function should return form here with error because if NULL pointer is passed to get_next_line function then it will dangerous at run time.

Patch is attached.

Thanks!!!
Comment 1 Ryan C. Gordon 2015-05-26 16:22:21 UTC
This patch is now https://hg.libsdl.org/SDL_image/rev/65f1bb7a3eb2, thanks!

--ryan.
Comment 2 Sylvain 2015-05-27 08:43:36 UTC
src == NULL is a valid input when loading from an array:

1168 SDL_Surface *IMG_ReadXPMFromArray(char **xpm)
1169 {
1170     return load_xpm(xpm, NULL);
1171 }


"get_next_line" deals with it by using first "lines":

 918 static char *get_next_line(char ***lines, SDL_RWops *src, int len)
 919 {
 920     char *linebufnew;
 921 
 922     if (lines) {
 923         return *(*lines)++;
 924     } else {
 925         char c;
 926         int n;
 927         do {
 928             if (SDL_RWread(src, &c, 1, 1) <= 0) {


What is actually incorrect is to have both "src" null and "lines" null.
That could be checked in "load_xpm" but it's cleaner to be done in "ReadXPMFromArray" (since it is already done in IMG_LoadXPM_RW):

1158 /* Load a XPM type image from an RWops datasource */
1159 SDL_Surface *IMG_LoadXPM_RW(SDL_RWops *src)
1160 {
1161     if ( !src ) {
1162         /* The error message has been set in SDL_RWFromFile */
1163         return NULL;
1164     }
1165     return load_xpm(NULL, src);
1166 }
1167 
1168 SDL_Surface *IMG_ReadXPMFromArray(char **xpm)
1169 {

+ if (!xpm) return NULL; //+ error message?

1170     return load_xpm(xpm, NULL);
1171 }
Comment 3 Ryan C. Gordon 2015-05-27 14:51:43 UTC
Backed out patch.

--ryan.
Comment 4 Ryan C. Gordon 2015-05-27 23:12:54 UTC
Better patch is now https://hg.libsdl.org/SDL_image/rev/bca82f1c65e1 ...

--ryan.