From owner-svn-src-head@freebsd.org Tue Sep 6 19:00:39 2016 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 1BD4FBC79B8; Tue, 6 Sep 2016 19:00:39 +0000 (UTC) (envelope-from emaste@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id C6607DD3; Tue, 6 Sep 2016 19:00:38 +0000 (UTC) (envelope-from emaste@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id u86J0bgj076629; Tue, 6 Sep 2016 19:00:37 GMT (envelope-from emaste@FreeBSD.org) Received: (from emaste@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id u86J0bd4076628; Tue, 6 Sep 2016 19:00:37 GMT (envelope-from emaste@FreeBSD.org) Message-Id: <201609061900.u86J0bd4076628@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: emaste set sender to emaste@FreeBSD.org using -f From: Ed Maste Date: Tue, 6 Sep 2016 19:00:37 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r305486 - head/usr.bin/bsdiff/bspatch X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 06 Sep 2016 19:00:39 -0000 Author: emaste Date: Tue Sep 6 19:00:37 2016 New Revision: 305486 URL: https://svnweb.freebsd.org/changeset/base/305486 Log: bspatch: add sanity checks on sizes to avoid integer overflow Note that this introduces an explicit 2GB limit, but this was already implicit in variable and function argument types. This is based on the "non-cryptanalytic attacks against freebsd update components" anonymous gist. Further refinement is planned. Reviewed by: allanjude, cem, kib Obtained from: anonymous gist MFC after: 3 days Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D7619 Modified: head/usr.bin/bsdiff/bspatch/bspatch.c Modified: head/usr.bin/bsdiff/bspatch/bspatch.c ============================================================================== --- head/usr.bin/bsdiff/bspatch/bspatch.c Tue Sep 6 18:53:17 2016 (r305485) +++ head/usr.bin/bsdiff/bspatch/bspatch.c Tue Sep 6 19:00:37 2016 (r305486) @@ -43,6 +43,7 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include #include #include @@ -98,8 +99,8 @@ int main(int argc, char *argv[]) char *directory, *namebuf; int cbz2err, dbz2err, ebz2err; int newfd, oldfd; - ssize_t oldsize, newsize; - ssize_t bzctrllen, bzdatalen; + off_t oldsize, newsize; + off_t bzctrllen, bzdatalen; u_char header[32], buf[8]; u_char *old, *new; off_t oldpos, newpos; @@ -194,7 +195,9 @@ int main(int argc, char *argv[]) bzctrllen = offtin(header + 8); bzdatalen = offtin(header + 16); newsize = offtin(header + 24); - if ((bzctrllen < 0) || (bzdatalen < 0) || (newsize < 0)) + if (bzctrllen < 0 || bzctrllen > OFF_MAX - 32 || + bzdatalen < 0 || bzctrllen + 32 > OFF_MAX - bzdatalen || + newsize < 0 || newsize > SSIZE_MAX) errx(1, "Corrupt patch\n"); /* Close patch file and re-open it via libbzip2 at the right places */ @@ -217,12 +220,13 @@ int main(int argc, char *argv[]) errx(1, "BZ2_bzReadOpen, bz2err = %d", ebz2err); if ((oldsize = lseek(oldfd, 0, SEEK_END)) == -1 || - (old = malloc(oldsize+1)) == NULL || + oldsize > SSIZE_MAX || + (old = malloc(oldsize)) == NULL || lseek(oldfd, 0, SEEK_SET) != 0 || read(oldfd, old, oldsize) != oldsize || close(oldfd) == -1) err(1, "%s", argv[1]); - if ((new = malloc(newsize + 1)) == NULL) + if ((new = malloc(newsize)) == NULL) err(1, NULL); oldpos = 0; @@ -238,7 +242,8 @@ int main(int argc, char *argv[]) } /* Sanity-check */ - if ((ctrl[0] < 0) || (ctrl[1] < 0)) + if (ctrl[0] < 0 || ctrl[0] > INT_MAX || + ctrl[1] < 0 || ctrl[1] > INT_MAX) errx(1, "Corrupt patch\n"); /* Sanity-check */