cipherpunk / google-security-research

Automatically exported from code.google.com/p/google-security-research
0 stars 0 forks source link

Avast: stack buffer overflow, strncpy length discarded #575

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
I can see that in strfn.cpp from the upstream unrar sources that the IntToExt() 
is defined like this:

void IntToExt(const char *Src,char *Dest,size_t DestSize)
{
#ifdef _WIN_ALL
  OemToCharBuffA(Src,Dest,(DWORD)DestSize);
  Dest[DestSize-1]=0;
#elif defined(_ANDROID)
  wchar DestW[NM];
  JniCharToWide(Src,DestW,ASIZE(DestW),true);
  WideToChar(DestW,Dest,DestSize);
#else
  if (Dest!=Src)
    strncpyz(Dest,Src,DestSize);
#endif
}

However, I can see in the disassembly of IntToExt() in Avast that the DestSize 
parameter is just discarded, and dep_strOemToAnsi() is called, which just 
invokes strcpy(). That seems like an stack buffer overflow vulnerability.

(gdb) disassemble IntToExt
Dump of assembler code for function _Z8IntToExtPKcPcj:
   0xf6f32520 <+0>: push   ebp
   0xf6f32521 <+1>: mov    ebp,esp
   0xf6f32523 <+3>: push   ebx
   0xf6f32524 <+4>: sub    esp,0x14
   0xf6f32527 <+7>: mov    eax,DWORD PTR [ebp+0xc]
   0xf6f3252a <+10>:    call   0xf6e67d69 <__i686.get_pc_thunk.bx>
   0xf6f3252f <+15>:    add    ebx,0x1a6c99
   0xf6f32535 <+21>:    mov    DWORD PTR [esp+0x4],eax
   0xf6f32539 <+25>:    mov    eax,DWORD PTR [ebp+0x8]
   0xf6f3253c <+28>:    mov    DWORD PTR [esp],eax
   0xf6f3253f <+31>:    call   0xf7021e50 <dep_strOemToAnsi>
   0xf6f32544 <+36>:    add    esp,0x14
   0xf6f32547 <+39>:    pop    ebx
   0xf6f32548 <+40>:    pop    ebp
   0xf6f32549 <+41>:    ret    
End of assembler dump.
(gdb) disassemble dep_strOemToAnsi
Dump of assembler code for function dep_strOemToAnsi:
   0xf7021e50 <+0>: push   ebp
   0xf7021e51 <+1>: mov    ebp,esp
   0xf7021e53 <+3>: push   ebx
   0xf7021e54 <+4>: sub    esp,0x14
   0xf7021e57 <+7>: mov    edx,DWORD PTR [ebp+0x8]
   0xf7021e5a <+10>:    mov    eax,DWORD PTR [ebp+0xc]
   0xf7021e5d <+13>:    call   0xf6e67d69 <__i686.get_pc_thunk.bx>
   0xf7021e62 <+18>:    add    ebx,0xb7366
   0xf7021e68 <+24>:    cmp    eax,edx
   0xf7021e6a <+26>:    je     0xf7021e78 <dep_strOemToAnsi+40>
   0xf7021e6c <+28>:    mov    DWORD PTR [esp+0x4],edx
   0xf7021e70 <+32>:    mov    DWORD PTR [esp],eax
   0xf7021e73 <+35>:    call   0xf6e67594 <strcpy@plt>
   0xf7021e78 <+40>:    add    esp,0x14
   0xf7021e7b <+43>:    xor    eax,eax
   0xf7021e7d <+45>:    pop    ebx
   0xf7021e7e <+46>:    pop    ebp
   0xf7021e7f <+47>:    ret    
End of assembler dump.

I'm confused how this could have happened, but regardless, this is obviously a 
very critical flaw.

This bug is subject to a 90 day disclosure deadline. If 90 days elapse
without a broadly available patch, then the bug report will automatically
become visible to the public.

Original issue reported on code.google.com by tav...@google.com on 13 Oct 2015 at 11:09

GoogleCodeExporter commented 8 years ago
I'll also email Eugene about not handleing DestSize=0 for the _WIN_ALL case.

Original comment by tav...@google.com on 13 Oct 2015 at 11:10

GoogleCodeExporter commented 8 years ago
Avast confirm this issue has been resolved.

Original comment by tav...@google.com on 14 Dec 2015 at 2:50