[Swftools-common] crash bug of swfstrings in swftools-0.9.2 caused by missing null pointer check of global static fonts array

[123yangke321]

Hi, swftools developers:
     We find a crash bug of swfstrings in swftools-0.9.2. The triggering 
command is:swfstrings exploit_0_0Here we provide the sample file, trace stack 
and crash spot.
...(-1)00 (0)56 (1)00 (2)13 (3)90 GetBits() out of bounds: TagID = 10, pos=4, 
len=4(-1)00 (0)56 (1)00 (2)13 (3)90 GetBits() out of bounds: TagID = 10, pos=4, 
len=4(-1)00 (0)56 (1)00 (2)13 (3)90 
Program received signal SIGSEGV, Segmentation fault.0x08049253 in textcallback 
(self=0xbfffe6d8, glyphs=0xbfffde70,     advance=0xbfffe270, nr=15, fontid=5, 
fontsize=960, startx=0, starty=1053,     color=0xbfffde44) at 
swfstrings.c:119119          if(fonts[t]->id == fontid) {(gdb) bt#0  0x08049253 
in textcallback (self=0xbfffe6d8, glyphs=0xbfffde70,     advance=0xbfffe270, 
nr=15, fontid=5, fontsize=960, startx=0, starty=1053,     color=0xbfffde44) at 
swfstrings.c:119#1  0x0804aba2 in swf_FontExtract_DefineTextCallback 
(address@hidden,     address@hidden, address@hidden, address@hidden,     
address@hidden <textcallback>,     address@hidden) at modules/swftext.c:516#2  
0x0804be9e in swf_ParseDefineText (address@hidden,     address@hidden 
<textcallback>,     address@hidden) at modules/swftext.c:527#3  0x08049d37 in 
main (argc=2, argv=0xbfffe804) at swfstrings.c:237(gdb) p t$1 = 1(gdb) p 
fonts$2 = (SWFFONT **) 0x80d4700(gdb) p fonts[1]$3 = (SWFFONT *) 0x0(gdb) p 
fonts[0]$4 = (SWFFONT *) 0x80d4710(gdb) l114       {115        SWFFONT*font = 
0;116            int t;117       for(t=0;t<fontnum;t++)118       {119           
     if(fonts[t]->id == fontid) {120             font = fonts[t];121            
 break;122           }123        }(gdb) p fontnum$5 = 3(gdb) p fonts[2]$6 = 
(SWFFONT *) 0x81ac900(gdb) 
Three element of fonts are assigned here, and the crash is caused by the second 
null element(textcallback  function failed to fetch its field:id) .
The second element of  fonts(static array defined at swfstrings.c) 
By examining the source code execution trace, we find the second element of 
fonts is assigned by swf_FontExtract.
629             t = swf_NextTag(t);swf_NextTag (address@hidden) at 
rfxswf.c:5959      TAG * swf_NextTag(TAG * t) { return t->next; 
}swf_FontExtract (address@hidden <swf>, address@hidden,    font=0x80d4704) at 
modules/swftext.c:593593         while (t) {595             switch 
(swf_GetTagID(t)) {swf_GetTagID (address@hidden) at rfxswf.c:6161      U16   
swf_GetTagID(TAG * t)    { return t->id; }swf_FontExtract (address@hidden 
<swf>, address@hidden,    font=0x80d4704) at modules/swftext.c:594594           
  int nid = 0;627             if (nid > 0)629             t = 
swf_NextTag(t);swf_NextTag (address@hidden) at rfxswf.c:5959      TAG * 
swf_NextTag(TAG * t) { return t->next; }swf_FontExtract (address@hidden <swf>, 
address@hidden,    font=0x80d4704) at modules/swftext.c:593593         while 
(t) {631         if (f->id != id) {632             rfx_free(f);rfx_free 
(address@hidden) at mem.c:1010      {11        if(!ptr)13        free(ptr);14   
   }swf_FontExtract (address@hidden <swf>, address@hidden,    font=0x80d4704) 
at modules/swftext.c:633633             f = 0;635         font[0] = f;636       
  return 0;637     }fontcallback2 (self=0x0, id=45159, name=0xbfffe57b 
"\t\261\064")    at swfstrings.c:109109       fontnum++;
The  call site of swf_FontExtract is at fontcallback2，
void fontcallback2(void*self, U16 id,U8 * name){   
swf_FontExtract(&swf,id,&fonts[fontnum]);  fontnum++;}
The parameter font corresponds to &fonts[fontnum] here in which fontnum=1.This 
is according to the definition of swf_FontExtract at lib/modules/swftext.c.
int swf_FontExtract(SWF * swf, int id, SWFFONT * *font)
This zero assignment of font is caused by error handling operation under 
condition " if (f->id != id) {".While this inequality is caused by former error 
signal value returned to nid.（as you can see in the formmer complain outputs of 
swfstrings“GetBits() out of bounds”）
int swf_FontExtract(SWF * swf, int id, SWFFONT * *font){    TAG *t;    SWFFONT 
*f;
    if ((!swf) || (!font))      return -1;
    f = (SWFFONT *) rfx_calloc(sizeof(SWFFONT));
    t = swf->firstTag;
    while (t) { int nid = 0;    switch (swf_GetTagID(t)) {      case 
ST_DEFINEFONT:         nid = swf_FontExtract_DefineFont(id, f, t);     break;
        case ST_DEFINEFONT2:    case ST_DEFINEFONT3:        nid = 
swf_FontExtract_DefineFont2(id, f, t);            break;
        case ST_DEFINEFONTALIGNZONES:       nid = 
swf_FontExtract_DefineFontAlignZones(id, f, t);           break;
        case ST_DEFINEFONTINFO: case ST_DEFINEFONTINFO2:            nid = 
swf_FontExtract_DefineFontInfo(id, f, t);         break;
        case ST_DEFINETEXT:     case ST_DEFINETEXT2:        if(!f->layout) {    
        nid = swf_FontExtract_DefineText(id, f, t, FEDTJ_MODIFY);           }   
    if(f->version>=3 && f->layout)              swf_FontUpdateUsage(f, t);      
    break;
        case ST_GLYPHNAMES:         nid = swf_FontExtract_GlyphNames(id, f, t); 
    break;      }       if (nid > 0)        id = nid;   t = swf_NextTag(t);    
}    if (f->id != id) {  rfx_free(f);    f = 0;    }    font[0] = f;    return 
0;}
We believe adding check code  before checking the id  of elements of fonts will 
hopefully fix this problem.
swfstrings.c:
void textcallback(void*self, int*glyphs, int*advance, int nr, int fontid, int 
fontsize, int startx, int starty, RGBA*color) {    SWFFONT*font = 0;    int t;  
  for(t=0;t<fontnum;t++)    {     /*Add additional null pointer check logic 
here*/        if(fonts[t]->id == fontid) {        font = fonts[t];        
break;      }    }    ...}
Thanks for your attention.
Ke Yang(杨克)
ossecurity group of Institute of Software Chinese Academy of 
Sciences(ISCAS)(系统安全性分析小组(系统威胁关联分析组)@中国科学院软件研究所）