This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

[Ada] Fix for buffer overflows in GNAT run-time library


The following patch removes a few potential buffer overflow bugs in
the Ada run-time library.  At least the bug in __gnat_tmp_name()
appears to be locally exploitable if a setuid Ada application uses the
standard interface for creating temporary files.

A genuine problem in win32_no_block_spawn() remains (the required
changes are not completely trivial, and I haven't got a development
environment to test it), and I didn't examine the VMS stuff (either
there are tons of problems, or the VMS APIs guarantee a limited string
length).

I think the remaining invocations of strcpy(), strcat() and sprintf()
are safe.


2001-10-20  Florian Weimer  <fw@deneb.enyo.de>

	* cstreams.c (__gnat_full_name): Eliminate risk of buffer
	overflow.

	* adaint.c (__gnat_try_lock): Increase buffer size, eliminate
	buffer overflow.
	(__gnat_tmp_name): Eliminate buffer overflow.
	(__gnat_readdir): Eliminate buffer overflow, readdir_r() case is
	still problematic.
	(win32_no_block_spawn): Add ??? marks because of buffer overflow.


Index: cstreams.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/ada/cstreams.c,v
retrieving revision 1.2
diff -c -3 -p -r1.2 cstreams.c
*** cstreams.c	2001/10/04 17:50:42	1.2
--- cstreams.c	2001/10/20 14:58:33
*************** __gnat_full_name (nam, buffer)
*** 225,231 ****
  #else
    if (nam[0] != '/')
      {
!       p = getcwd (buffer, max_path_len);
        if (p == 0)
  	{
  	  buffer[0] = '\0';
--- 225,231 ----
  #else
    if (nam[0] != '/')
      {
!       p = getcwd (buffer, max_path_len - (strlen(nam) + 1));
        if (p == 0)
  	{
  	  buffer[0] = '\0';
*************** __gnat_full_name (nam, buffer)
*** 239,247 ****
  
        strcat (buffer, nam);
      }
!   else
!     strcpy (buffer, nam);
! 
    return buffer;
  #endif
  }
--- 239,254 ----
  
        strcat (buffer, nam);
      }
!   else 
!     {
!       if (strlen(nam) < max_path_len)
! 	  strcpy (buffer, nam);
!       else 
! 	{
! 	  buffer[0] = '\0';
! 	  return 0;
! 	}
!     }
    return buffer;
  #endif
  }
Index: adaint.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/ada/adaint.c,v
retrieving revision 1.2
diff -c -3 -p -r1.2 adaint.c
*** adaint.c	2001/10/04 17:50:42	1.2
--- adaint.c	2001/10/20 14:58:35
*************** __gnat_try_lock (dir, file)
*** 244,253 ****
       char *dir;
       char *file;
  {
-   char full_path [256];
    int fd;
  
!   sprintf (full_path, "%s%c%s", dir, DIR_SEPARATOR, file);
    fd = open (full_path, O_CREAT | O_EXCL, 0600);
    if (fd < 0) {
      return 0;
--- 244,253 ----
       char *dir;
       char *file;
  {
    int fd;
+   char full_path [1024];
  
!   sprintf (full_path, "%.500s%c%.500s", dir, DIR_SEPARATOR, file);
    fd = open (full_path, O_CREAT | O_EXCL, 0600);
    if (fd < 0) {
      return 0;
*************** __gnat_try_lock (dir, file)
*** 266,275 ****
       char *dir;
       char *file;
  {
-   char full_path [256];
    int fd;
  
!   sprintf (full_path, "%s%c%s", dir, DIR_SEPARATOR, file);
    fd = open (full_path, O_CREAT | O_EXCL, 0600);
    if (fd < 0)
      return 0;
--- 266,275 ----
       char *dir;
       char *file;
  {
    int fd;
+   char full_path [1024];
  
!   sprintf (full_path, "%.500s%c%.500s", dir, DIR_SEPARATOR, file);
    fd = open (full_path, O_CREAT | O_EXCL, 0600);
    if (fd < 0)
      return 0;
*************** __gnat_try_lock (dir, file)
*** 286,298 ****
       char *dir;
       char *file;
  {
-   char full_path [256];
-   char temp_file [256];
    struct stat stat_result;
    int fd;
  
!   sprintf (full_path, "%s%c%s", dir, DIR_SEPARATOR, file);
!   sprintf (temp_file, "%s-%d-%d", dir, getpid(), getppid ());
  
    /* Create the temporary file and write the process number */
    fd = open (temp_file, O_CREAT | O_WRONLY, 0600);
--- 286,298 ----
       char *dir;
       char *file;
  {
    struct stat stat_result;
    int fd;
+   char full_path [1024];
+   char temp_file [1024];
  
!   sprintf (full_path, "%.500s%c%.500s", dir, DIR_SEPARATOR, file);
!   sprintf (temp_file, "%.500s-%d-%d", dir, getpid(), getppid ());
  
    /* Create the temporary file and write the process number */
    fd = open (temp_file, O_CREAT | O_WRONLY, 0600);
*************** __gnat_file_length (fd)
*** 627,633 ****
  }
  
  /* Create a temporary filename and put it in string pointed to by
!    tmp_filename */
  
  void
  __gnat_tmp_name (tmp_filename)
--- 627,634 ----
  }
  
  /* Create a temporary filename and put it in string pointed to by
!    tmp_filename.  tmp_filename must point to at least max_path_len 
!    characters.  */
  
  void
  __gnat_tmp_name (tmp_filename)
*************** __gnat_tmp_name (tmp_filename)
*** 660,666 ****
  #elif defined (linux)
    char *tmpdir = getenv ("TMPDIR");
  
!   if (tmpdir == NULL)
      strcpy (tmp_filename, "/tmp/gnat-XXXXXX");
    else
      sprintf (tmp_filename, "%s/gnat-XXXXXX", tmpdir);
--- 661,667 ----
  #elif defined (linux)
    char *tmpdir = getenv ("TMPDIR");
  
!   if (tmpdir == NULL || (strlen(tmpdir) + 20) > (size_t) max_path_len)
      strcpy (tmp_filename, "/tmp/gnat-XXXXXX");
    else
      sprintf (tmp_filename, "%s/gnat-XXXXXX", tmpdir);
*************** __gnat_readdir (dirp, buffer)
*** 681,686 ****
--- 682,688 ----
  {
    /* If possible, try to use the thread-safe version.  */
  #ifdef HAVE_READDIR_R
+   /* ??? readdir_r() might overflow buffer. */
    if (readdir_r (dirp, buffer) != NULL)
      return ((struct dirent*) buffer)->d_name;
    else
*************** __gnat_readdir (dirp, buffer)
*** 689,695 ****
  #else
    struct dirent *dirent = readdir (dirp);
  
!   if (dirent != NULL)
      {
        strcpy (buffer, dirent->d_name);
        return buffer;
--- 691,697 ----
  #else
    struct dirent *dirent = readdir (dirp);
  
!   if (dirent != NULL && strlen(dirent->d_name) <= 1024)
      {
        strcpy (buffer, dirent->d_name);
        return buffer;
*************** win32_no_block_spawn (command, args)
*** 1293,1299 ****
    PROCESS_INFORMATION PI;
    SECURITY_ATTRIBUTES SA;
  
!   char full_command [2000];
    int k;
  
    /* Startup info. */
--- 1295,1301 ----
    PROCESS_INFORMATION PI;
    SECURITY_ATTRIBUTES SA;
  
!   char full_command [2000];	/* ??? buffer overflow bug */
    int k;
  
    /* Startup info. */


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]