Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/usr.bin/gzip Coverity CID 1264915, Via FreeBSD (Xin Li)



details:   https://anonhg.NetBSD.org/src/rev/84a769d09869
branches:  trunk
changeset: 337455:84a769d09869
user:      christos <christos%NetBSD.org@localhost>
date:      Wed Apr 15 02:29:12 2015 +0000

description:
Coverity CID 1264915, Via FreeBSD (Xin Li)

When reading in the original file name from gzip header, we read
in PATH_MAX + 1 bytes from the file.  In r281500, strrchr() is
used to strip possible path portion of the file name to mitigate
a possible attack.  Unfortunately, strrchr() expects a buffer
that is NUL-terminated, and since we are processing potentially
untrusted data, we can not assert that be always true.

Solve this by reading in one less byte (now PATH_MAX) and
explicitly terminate the buffer after the read size with NUL.

diffstat:

 usr.bin/gzip/gzip.c |  11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diffs (38 lines):

diff -r 60cde5c46045 -r 84a769d09869 usr.bin/gzip/gzip.c
--- a/usr.bin/gzip/gzip.c       Tue Apr 14 22:36:53 2015 +0000
+++ b/usr.bin/gzip/gzip.c       Wed Apr 15 02:29:12 2015 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: gzip.c,v 1.107 2015/01/13 02:37:20 mrg Exp $   */
+/*     $NetBSD: gzip.c,v 1.108 2015/04/15 02:29:12 christos Exp $      */
 
 /*
  * Copyright (c) 1997, 1998, 2003, 2004, 2006 Matthew R. Green
@@ -30,7 +30,7 @@
 #ifndef lint
 __COPYRIGHT("@(#) Copyright (c) 1997, 1998, 2003, 2004, 2006\
  Matthew R. Green.  All rights reserved.");
-__RCSID("$NetBSD: gzip.c,v 1.107 2015/01/13 02:37:20 mrg Exp $");
+__RCSID("$NetBSD: gzip.c,v 1.108 2015/04/15 02:29:12 christos Exp $");
 #endif /* not lint */
 
 /*
@@ -1366,14 +1366,17 @@
                timestamp = ts[3] << 24 | ts[2] << 16 | ts[1] << 8 | ts[0];
 
                if (header1[3] & ORIG_NAME) {
-                       rbytes = pread(fd, name, sizeof name, GZIP_ORIGNAME);
+                       rbytes = pread(fd, name, sizeof(name) - 1, GZIP_ORIGNAME);
                        if (rbytes < 0) {
                                maybe_warn("can't read %s", file);
                                goto lose;
                        }
-                       if (name[0] != 0) {
+                       if (name[0] != '\0') {
                                char *dp, *nf;
 
+                               /* Make sure that name is NUL-terminated */
+                               name[rbytes] = '\0';
+
                                /* strip saved directory name */
                                nf = strrchr(name, '/');
                                if (nf == NULL)



Home | Main Index | Thread Index | Old Index