[LTP] [PATCH] Fix buffer overflow in print_result() function
Veronika Kabatova
vkabatov@redhat.com
Tue Nov 7 16:35:32 CET 2017
----- Original Message -----
> From: "Cyril Hrubis" <chrubis@suse.cz>
> To: vkabatov@redhat.com
> Cc: ltp@lists.linux.it
> Sent: Monday, November 6, 2017 4:00:58 PM
> Subject: Re: [LTP] [PATCH] Fix buffer overflow in print_result() function
>
> Hi!
> > lib/tst_test.c | 18 ++++++++++++++++--
> > 1 file changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/tst_test.c b/lib/tst_test.c
> > index c8baf2a43..09691031e 100644
> > --- a/lib/tst_test.c
> > +++ b/lib/tst_test.c
> > @@ -180,7 +180,7 @@ static void print_result(const char *file, const int
> > lineno, int ttype,
> > {
> > char buf[1024];
> > char *str = buf;
> > - int ret, size = sizeof(buf);
> > + int ret, overflowed = 0, size = sizeof(buf);
> > const char *str_errno = NULL;
> > const char *res;
> >
> > @@ -227,17 +227,31 @@ static void print_result(const char *file, const int
> > lineno, int ttype,
> > size -= ret;
> >
> > ret = vsnprintf(str, size, fmt, va);
> > + if (ret >= size) {
> > + overflowed = 1;
> > + goto finish;
> > + }
> > str += ret;
> > size -= ret;
> >
> > if (str_errno) {
> > ret = snprintf(str, size, ": %s", str_errno);
> > + if (ret >= size) {
> > + overflowed = 1;
> > + goto finish;
> > + }
> > str += ret;
> > size -= ret;
> > }
>
> We can simplify this a bit I guess.
>
> We may as well pass size-2 to the snprintf() functions here, then add
> MIN(ret, size-2) to the str. Then we don't have to use the overflowed
> variable since the str would point to the end of the composed string
> and there would be always at least two bytes in the buffer so that the
> last one can be just sprintf() or strcpy().
>
> > - snprintf(str, size, "\n");
> > +finish:
> > + /* Keep space for newline and \0 if the buffer was filled */
> > + if (overflowed) {
> > + str += size - 2;
> > + size = 2;
> > + }
> >
> > + snprintf(str, size, "\n");
> > fputs(buf, stderr);
>
> What about printing TWARN message here in a case that the message was
> shortened, something as tst_res_(file, lineno, TWARN, "Previous message was
> too long!"),
> we would have to keep the overflow flag for that thought...
>
Hi, I like this idea. I'll rewrite it differently so we don't need to keep
the flag and also include the MIN() macro you mentioned above.
> > }
> >
> > --
> > 2.13.6
> >
> >
> > --
> > Mailing list info: https://lists.linux.it/listinfo/ltp
>
> --
> Cyril Hrubis
> chrubis@suse.cz
>
Veronika Kabatova
More information about the ltp
mailing list