[libgfortran] Bug 81937 - stack-buffer-overflow on memcpy

Message ID 34841158-f941-5ed9-d923-a7197d72105f@charter.net
State New
Headers show
Series
  • [libgfortran] Bug 81937 - stack-buffer-overflow on memcpy
Related show

Commit Message

Jerry Dec. 16, 2017, 4:26 p.m.
Hi all,

This problem was found with -fsanitize=address.

Turns out we are not correctly tracking the bytes left in the internal unit
string and we were reading memory past the end. I am sure the problem exists in
gcc 7 and I will examine gcc 6 as well and fix this in all cases I see. The
function sread is basically a wrapper on memcpy

The patch is fairly straight forward.

Regression tested on x86_64-pc-linux-gnu. OK for trunk and back ports as I find?

Regards,

Jerry

2017-12-16  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

	PR libgfortran/81937
	* io/list_read.c (next_char_internal): Don't attempt to read
	from the internal unit stream if no bytes are left. Decrement
	bytes_left in the right place.

Comments

Janne Blomqvist Dec. 16, 2017, 7:37 p.m. | #1
On Sat, Dec 16, 2017 at 6:26 PM, Jerry DeLisle <jvdelisle@charter.net> wrote:
> Hi all,

>

> This problem was found with -fsanitize=address.

>

> Turns out we are not correctly tracking the bytes left in the internal unit

> string and we were reading memory past the end. I am sure the problem exists in

> gcc 7 and I will examine gcc 6 as well and fix this in all cases I see. The

> function sread is basically a wrapper on memcpy

>

> The patch is fairly straight forward.

>

> Regression tested on x86_64-pc-linux-gnu. OK for trunk and back ports as I find?

>

> Regards,

>

> Jerry

>

> 2017-12-16  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

>

>         PR libgfortran/81937

>         * io/list_read.c (next_char_internal): Don't attempt to read

>         from the internal unit stream if no bytes are left. Decrement

>         bytes_left in the right place.

>


Looks good, Ok for trunk/7/6. Thanks!

-- 
Janne Blomqvist

Patch

diff --git a/libgfortran/io/list_read.c b/libgfortran/io/list_read.c
index 379050cecad..037f2daa647 100644
--- a/libgfortran/io/list_read.c
+++ b/libgfortran/io/list_read.c
@@ -266,15 +266,19 @@  next_char_internal (st_parameter_dt *dtp)
     }
 
   /* Get the next character and handle end-of-record conditions.  */
-
-  if (is_char4_unit(dtp)) /* Check for kind=4 internal unit.  */
-   length = sread (dtp->u.p.current_unit->s, &c, 1);
+  if (likely (dtp->u.p.current_unit->bytes_left > 0))
+    {
+      if (unlikely (is_char4_unit(dtp))) /* Check for kind=4 internal unit.  */
+       length = sread (dtp->u.p.current_unit->s, &c, 1);
+      else
+       {
+	 char cc;
+	 length = sread (dtp->u.p.current_unit->s, &cc, 1);
+	 c = cc;
+       }
+    }
   else
-   {
-     char cc;
-     length = sread (dtp->u.p.current_unit->s, &cc, 1);
-     c = cc;
-   }
+    length = 0;
 
   if (unlikely (length < 0))
     {
@@ -290,7 +294,6 @@  next_char_internal (st_parameter_dt *dtp)
 	  generate_error (&dtp->common, LIBERROR_INTERNAL_UNIT, NULL);
 	  return '\0';
 	}
-      dtp->u.p.current_unit->bytes_left--;
     }
   else
     {
@@ -302,6 +305,7 @@  next_char_internal (st_parameter_dt *dtp)
 	  dtp->u.p.at_eof = 1;
 	}
     }
+  dtp->u.p.current_unit->bytes_left--;
 
 done:
   dtp->u.p.at_eol = (c == '\n' || c == EOF);