| Message ID | 34841158-f941-5ed9-d923-a7197d72105f@charter.net |
|---|---|
| State | New |
| Headers | show |
| Series | [libgfortran] Bug 81937 - stack-buffer-overflow on memcpy | expand |
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!
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);