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

Jeffrey Walton INVALID.NOREPLY at gnu.org
Fri Nov 19 04:03:43 CET 2010


                 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



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],
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

>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

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:

    *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;

    *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



Reply to this item at:


  Message sent via/by Savannah

More information about the Gnutls-devel mailing list