[sr #107527] Use of dangerous/banned functions (Analysis)

[Jeffrey Walton]

URL:
  <http://savannah.gnu.org/support/?107527>

                 Summary: Use of dangerous/banned functions (Analysis)
                 Project: GnuTLS
            Submitted by: noloader
            Submitted on: Fri 19 Nov 2010 03:03:42 AM GMT
                Category: None
                Priority: 5 - Normal
                Severity: 3 - Normal
                  Status: None
                 Privacy: Public
             Assigned to: None
        Originator Email: 
             Open/Closed: Open
         Discussion Lock: Any
        Operating System: None

    _______________________________________________________

Details:

SR #107522 flagged the general use of unsafe string handling functions. SR
#107525 detailed particular instances of the use of unsafe string handling
functions; and highlighted GnuTLS's inability to detect failure.

This SR will analyze the code around a cluster of calls to strcpy in serv.c.
The lines of interest in peer_print_info (from the SR $107525 audit) are:

451: size_t len = 5 * 1024 + strlen (header);
457: http_buffer = malloc (len);
...

461: strcpy (http_buffer, HTTP_BEGIN);
462: strcpy (&http_buffer[sizeof (HTTP_BEGIN) - 1], DEFAULT_DATA);
463: strcpy (&http_buffer[sizeof (HTTP_BEGIN) + sizeof (DEFAULT_DATA) - 2],
HTTP_END);
465: *ret_length = sizeof (DEFAULT_DATA) + sizeof (HTTP_BEGIN) + sizeof
(HTTP_END) - 3;

=====
header, ehich is subsequently used in a call to strlen, is not checked for
validity. In addition, there does not appear to be documentation for
peer_print_info, so its unclear if Null/Empty values are acceptable to the
function.

>From the Open Group Base Specifications for strlen(), it appears no
constraints are placed on the string 's' and no errors are defined for the
function, which implies all arguments - including NULL - are acceptable. Yet a
test program faults on GNU Linux with a NULL argument:

Program received signal SIGSEGV, Segmentation fault.
__strlen_sse2 () at ../sysdeps/x86_64/multiarch/../strlen.S:31
31	../sysdeps/x86_64/multiarch/../strlen.S: No such file or directory.
	in ../sysdeps/x86_64/multiarch/../strlen.S
(gdb) bt full
#0  __strlen_sse2 () at ../sysdeps/x86_64/multiarch/../strlen.S:31
No locals.
#1  0x000000000040066b in peer_print_info_kindof (header=0x0, 
    ret_length=0x7fffffffe2ac) at replace-strcpy.c:43
        http_buffer = 0x400755 "H\205\355t\034\061\333\017\037@"
        len = 140737488348072
#2  0x000000000040062b in main (argc=1, argv=0x7fffffffe398)
    at replace-strcpy.c:14
        written = 0
        response = 0x7fffffffe390 "\001"

(Sorry for not checking ISO C - I don't have a licensed copy of the
standard).

=====
The header in line 451 is passed in to peer_print_info. Based on the header
length, a buffer is allocated. The 5 * 1024 appears to be a heuristic.

=====
At line 457, memory is obtained for a HTTP response. The call to malloc is
checked for failure.

=====
The strcpy's at lines 461, 462, and 463 assume success since the function
cannot convey failures. The assumption is then propagated to the caller via
the dereference/assignment at line 465.

=====
The dereference at line 465 assumes the pointer is valid.

=====
If the invocation of peer_print_info is unsuccessful, ret_length could be in
an indeterminate state from the caller's perspective.

=====
An improved strategy might include changing calls to strcpy with calls to
snprintf since snprintf offers deterministic failure:

  if(ret_length)
    *ret_length = -1;

  size_t len = 5 * 1024 + strlen (header);
  ...

  size_t len2 = 0; /* scratch */
  int count = 0, written = 0;

  len2 = strlen(DEFAULT_DATA);
  if(!(len - written >= len2)) { /*handle failure*/ }

  count = snprintf(&http_buffer[written], len-written, "%s", DEFAULT_DATA);
  if(count < 0) { /*handle failure*/ }
  written += count;
  ...

  if(ret_length)
    *ret_length = written;



    _______________________________________________________

File Attachments:


-------------------------------------------------------
Date: Fri 19 Nov 2010 03:03:42 AM GMT  Name: peer_print_info_kindof.c  Size:
3kB   By: noloader

<http://savannah.gnu.org/support/download.php?file_id=22057>

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/support/?107527>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/