Menu

#1262 curl_formadd() with CURLFORM_BUFFERPTR results in invalid read when buffer has no zero bytes

closed-fixed
5
2014-12-27
2013-08-04
No

When the following program is run through valgrind, it states an error:

#include <stdlib.h>
#include <string.h>
#include <curl/curl.h>

int main()
{
  struct curl_httppost * post = NULL;
  struct curl_httppost * last = NULL;
  char * request = malloc( 4096 );
  memset( request, 1, 4096 );
#ifdef HIDE_BUG
  /* This would make the error go away - apparently strlen() is used on buffer */
  request[ 4095 ] = 0;
#endif
  curl_formadd( &post, &last,
                CURLFORM_COPYNAME, "a",
                CURLFORM_BUFFER, "b",
                CURLFORM_BUFFERPTR,  request,
                CURLFORM_BUFFERLENGTH, ( long ) 4096,
                CURLFORM_END );
  return 0;
}

$ gcc test.c -lcurl; valgrind ./a.out

==18321== Invalid read of size 1
==18321==    at 0x4C2B4F4: strlen (mc_replace_strmem.c:390)
==18321==    by 0x4E3FF07: curl_formadd (in /usr/lib/x86_64-linux-gnu/libcurl.so.4.3.0)
==18321==    by 0x400789: main (in /tmp/a.out)
==18321==  Address 0x887c0b0 is 0 bytes after a block of size 4,096 alloc'd
==18321==    at 0x4C2ABED: malloc (vg_replace_malloc.c:263)
==18321==    by 0x40071B: main (in /tmp/a.out)

$ curl -V

curl 7.31.0 (x86_64-pc-linux-gnu) libcurl/7.31.0 OpenSSL/1.0.1c zlib/1.2.7 libidn/1.25 libssh2/1.4.3 librtmp/2.3
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtmp rtsp scp sftp smtp smtps telnet tftp
Features: GSS-Negotiate IDN IPv6 Largefile NTLM NTLM_WB SSL libz TLS-SRP

Discussion

  • Daniel Stenberg

    Daniel Stenberg - 2013-08-04

    Thanks for the report. I'll look into it!

     
  • Daniel Stenberg

    Daniel Stenberg - 2013-08-04
    • labels: --> curl_formadd
    • assigned_to: Daniel Stenberg
     
  • Daniel Stenberg

    Daniel Stenberg - 2013-08-04
    • status: open --> open-confirmed
     
  • Daniel Stenberg

    Daniel Stenberg - 2013-08-04

    Thanks a lot for your excellent report and guidance on how to repeat the problem. This bug is now fixed in commit 0ddc678927eaa: https://github.com/bagder/curl/commit/0ddc678927eaa127efc457535858c19e791a5339

    I'm closing this report now!

     
  • Daniel Stenberg

    Daniel Stenberg - 2013-08-04
    • status: open-confirmed --> closed-fixed
     
  • Konstantin Isakov

    You're welcome! I was wondering though what a proper workaround for the existing versions of libcurl would be?

     
  • Daniel Stenberg

    Daniel Stenberg - 2013-08-04

    I think zero terminating the data pointed to so that libcurl won't read outside the allocated boundary. Setting a content-type could also work.

    As can be seen in the patch, the problem is/was that the buffer pointer was wrongly passed in to the function that checks the file name for a known extension to get a default content-type (if no custom such has been set). That function uses strlen() on the pointer.