Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 22 Sep 2016 21:05:21 +0000 (UTC)
From:      Ed Maste <emaste@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org
Subject:   svn commit: r306213 - stable/11/usr.bin/bsdiff/bspatch
Message-ID:  <201609222105.u8ML5LLe093825@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: emaste
Date: Thu Sep 22 21:05:21 2016
New Revision: 306213
URL: https://svnweb.freebsd.org/changeset/base/306213

Log:
  MFC bspatch Capsicumization, sanity checks, and other improvements
  
  r304691: bspatch: apply style(9)
  
  Make style changes (and trivial refactoring of open calls) now in order
  to reduce noise in diffs for future capsicum changes.
  
  r304807 (allanjude): Capsicumize bspatch
  
  Move all of the fopen() and open() calls to the top of main()
  
  Restrict each FD to least privilege (read/seek only, write only, etc)
  
  cap_enter(), and make all except the output FD read/seek only.
  
  r304821: bspatch: remove output file in the case of error
  
  r305486: 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.
  
  r305737: bspatch: remove superfluous newlines from errx strings
  
  r305822: bspatch: use #define for header size instead of magic number
  
  r306026: bspatch: Remove backwards-compatibility sys/capability.h support
  
  bspatch previously included sys/capability.h or sys/capsicum.h based
  on __FreeBSD_version, as FreeBSD is the upstream for bsdiff and we may
  see this file incorporated into other third-party software.
  
  The Capsicum header is now installed as sys/capsicum.h in stable/10 and
  FreeBSD 10.3, so we can just use sys/capsicum.h and simplify the logic.

Modified:
  stable/11/usr.bin/bsdiff/bspatch/bspatch.c
Directory Properties:
  stable/11/   (props changed)

Modified: stable/11/usr.bin/bsdiff/bspatch/bspatch.c
==============================================================================
--- stable/11/usr.bin/bsdiff/bspatch/bspatch.c	Thu Sep 22 20:34:44 2016	(r306212)
+++ stable/11/usr.bin/bsdiff/bspatch/bspatch.c	Thu Sep 22 21:05:21 2016	(r306213)
@@ -27,34 +27,60 @@
 #include <sys/cdefs.h>
 __FBSDID("$FreeBSD$");
 
+#if defined(__FreeBSD__)
+#include <sys/param.h>
+#if __FreeBSD_version >= 1001511
+#include <sys/capsicum.h>
+#define HAVE_CAPSICUM
+#endif
+#endif
+
 #include <bzlib.h>
-#include <stdlib.h>
+#include <err.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <libgen.h>
+#include <limits.h>
+#include <stdint.h>
 #include <stdio.h>
+#include <stdlib.h>
 #include <string.h>
-#include <err.h>
 #include <unistd.h>
-#include <fcntl.h>
 
 #ifndef O_BINARY
 #define O_BINARY 0
 #endif
+#define HEADER_SIZE 32
+
+static char *newfile;
+static int dirfd = -1;
+
+static void
+exit_cleanup(void)
+{
+
+	if (dirfd != -1 && newfile != NULL)
+		if (unlinkat(dirfd, newfile, 0))
+			warn("unlinkat");
+}
 
 static off_t offtin(u_char *buf)
 {
 	off_t y;
 
-	y=buf[7]&0x7F;
-	y=y*256;y+=buf[6];
-	y=y*256;y+=buf[5];
-	y=y*256;y+=buf[4];
-	y=y*256;y+=buf[3];
-	y=y*256;y+=buf[2];
-	y=y*256;y+=buf[1];
-	y=y*256;y+=buf[0];
+	y = buf[7] & 0x7F;
+	y = y * 256; y += buf[6];
+	y = y * 256; y += buf[5];
+	y = y * 256; y += buf[4];
+	y = y * 256; y += buf[3];
+	y = y * 256; y += buf[2];
+	y = y * 256; y += buf[1];
+	y = y * 256; y += buf[0];
 
-	if(buf[7]&0x80) y=-y;
+	if (buf[7] & 0x80)
+		y = -y;
 
-	return y;
+	return (y);
 }
 
 static void
@@ -65,20 +91,23 @@ usage(void)
 	exit(1);
 }
 
-int main(int argc,char * argv[])
+int main(int argc, char *argv[])
 {
-	FILE * f, * cpf, * dpf, * epf;
-	BZFILE * cpfbz2, * dpfbz2, * epfbz2;
+	FILE *f, *cpf, *dpf, *epf;
+	BZFILE *cpfbz2, *dpfbz2, *epfbz2;
+	char *directory, *namebuf;
 	int cbz2err, dbz2err, ebz2err;
-	int fd;
-	ssize_t oldsize,newsize;
-	ssize_t bzctrllen,bzdatalen;
-	u_char header[32],buf[8];
+	int newfd, oldfd;
+	off_t oldsize, newsize;
+	off_t bzctrllen, bzdatalen;
+	u_char header[HEADER_SIZE], buf[8];
 	u_char *old, *new;
-	off_t oldpos,newpos;
+	off_t oldpos, newpos;
 	off_t ctrl[3];
-	off_t lenread;
-	off_t i;
+	off_t i, lenread, offset;
+#ifdef HAVE_CAPSICUM
+	cap_rights_t rights_dir, rights_ro, rights_wr;
+#endif
 
 	if (argc != 4)
 		usage();
@@ -86,6 +115,54 @@ int main(int argc,char * argv[])
 	/* Open patch file */
 	if ((f = fopen(argv[3], "rb")) == NULL)
 		err(1, "fopen(%s)", argv[3]);
+	/* Open patch file for control block */
+	if ((cpf = fopen(argv[3], "rb")) == NULL)
+		err(1, "fopen(%s)", argv[3]);
+	/* open patch file for diff block */
+	if ((dpf = fopen(argv[3], "rb")) == NULL)
+		err(1, "fopen(%s)", argv[3]);
+	/* open patch file for extra block */
+	if ((epf = fopen(argv[3], "rb")) == NULL)
+		err(1, "fopen(%s)", argv[3]);
+	/* open oldfile */
+	if ((oldfd = open(argv[1], O_RDONLY | O_BINARY, 0)) < 0)
+		err(1, "open(%s)", argv[1]);
+	/* open directory where we'll write newfile */
+	if ((namebuf = strdup(argv[2])) == NULL ||
+	    (directory = dirname(namebuf)) == NULL ||
+	    (dirfd = open(directory, O_DIRECTORY)) < 0)
+		err(1, "open %s", argv[2]);
+	free(namebuf);
+	if ((newfile = basename(argv[2])) == NULL)
+		err(1, "basename");
+	/* open newfile */
+	if ((newfd = openat(dirfd, newfile,
+	    O_CREAT | O_TRUNC | O_WRONLY | O_BINARY, 0666)) < 0)
+		err(1, "open(%s)", argv[2]);
+	atexit(exit_cleanup);
+
+#ifdef HAVE_CAPSICUM
+	if (cap_enter() < 0) {
+		/* Failed to sandbox, fatal if CAPABILITY_MODE enabled */
+		if (errno != ENOSYS)
+			err(1, "failed to enter security sandbox");
+	} else {
+		/* Capsicum Available */
+		cap_rights_init(&rights_ro, CAP_READ, CAP_FSTAT, CAP_SEEK);
+		cap_rights_init(&rights_wr, CAP_WRITE);
+		cap_rights_init(&rights_dir, CAP_UNLINKAT);
+
+		if (cap_rights_limit(fileno(f), &rights_ro) < 0 ||
+		    cap_rights_limit(fileno(cpf), &rights_ro) < 0 ||
+		    cap_rights_limit(fileno(dpf), &rights_ro) < 0 ||
+		    cap_rights_limit(fileno(epf), &rights_ro) < 0 ||
+		    cap_rights_limit(oldfd, &rights_ro) < 0 ||
+		    cap_rights_limit(newfd, &rights_wr) < 0 ||
+		    cap_rights_limit(dirfd, &rights_dir) < 0)
+			err(1, "cap_rights_limit() failed, could not restrict"
+			    " capabilities");
+	}
+#endif
 
 	/*
 	File format:
@@ -102,99 +179,99 @@ int main(int argc,char * argv[])
 	*/
 
 	/* Read header */
-	if (fread(header, 1, 32, f) < 32) {
+	if (fread(header, 1, HEADER_SIZE, f) < HEADER_SIZE) {
 		if (feof(f))
-			errx(1, "Corrupt patch\n");
+			errx(1, "Corrupt patch");
 		err(1, "fread(%s)", argv[3]);
 	}
 
 	/* Check for appropriate magic */
 	if (memcmp(header, "BSDIFF40", 8) != 0)
-		errx(1, "Corrupt patch\n");
+		errx(1, "Corrupt patch");
 
 	/* Read lengths from header */
-	bzctrllen=offtin(header+8);
-	bzdatalen=offtin(header+16);
-	newsize=offtin(header+24);
-	if((bzctrllen<0) || (bzdatalen<0) || (newsize<0))
-		errx(1,"Corrupt patch\n");
+	bzctrllen = offtin(header + 8);
+	bzdatalen = offtin(header + 16);
+	newsize = offtin(header + 24);
+	if (bzctrllen < 0 || bzctrllen > OFF_MAX - HEADER_SIZE ||
+	    bzdatalen < 0 || bzctrllen + HEADER_SIZE > OFF_MAX - bzdatalen ||
+	    newsize < 0 || newsize > SSIZE_MAX)
+		errx(1, "Corrupt patch");
 
 	/* Close patch file and re-open it via libbzip2 at the right places */
 	if (fclose(f))
 		err(1, "fclose(%s)", argv[3]);
-	if ((cpf = fopen(argv[3], "rb")) == NULL)
-		err(1, "fopen(%s)", argv[3]);
-	if (fseeko(cpf, 32, SEEK_SET))
-		err(1, "fseeko(%s, %lld)", argv[3],
-		    (long long)32);
+	offset = HEADER_SIZE;
+	if (fseeko(cpf, offset, SEEK_SET))
+		err(1, "fseeko(%s, %jd)", argv[3], (intmax_t)offset);
 	if ((cpfbz2 = BZ2_bzReadOpen(&cbz2err, cpf, 0, 0, NULL, 0)) == NULL)
 		errx(1, "BZ2_bzReadOpen, bz2err = %d", cbz2err);
-	if ((dpf = fopen(argv[3], "rb")) == NULL)
-		err(1, "fopen(%s)", argv[3]);
-	if (fseeko(dpf, 32 + bzctrllen, SEEK_SET))
-		err(1, "fseeko(%s, %lld)", argv[3],
-		    (long long)(32 + bzctrllen));
+	offset += bzctrllen;
+	if (fseeko(dpf, offset, SEEK_SET))
+		err(1, "fseeko(%s, %jd)", argv[3], (intmax_t)offset);
 	if ((dpfbz2 = BZ2_bzReadOpen(&dbz2err, dpf, 0, 0, NULL, 0)) == NULL)
 		errx(1, "BZ2_bzReadOpen, bz2err = %d", dbz2err);
-	if ((epf = fopen(argv[3], "rb")) == NULL)
-		err(1, "fopen(%s)", argv[3]);
-	if (fseeko(epf, 32 + bzctrllen + bzdatalen, SEEK_SET))
-		err(1, "fseeko(%s, %lld)", argv[3],
-		    (long long)(32 + bzctrllen + bzdatalen));
+	offset += bzdatalen;
+	if (fseeko(epf, offset, SEEK_SET))
+		err(1, "fseeko(%s, %jd)", argv[3], (intmax_t)offset);
 	if ((epfbz2 = BZ2_bzReadOpen(&ebz2err, epf, 0, 0, NULL, 0)) == NULL)
 		errx(1, "BZ2_bzReadOpen, bz2err = %d", ebz2err);
 
-	if(((fd=open(argv[1],O_RDONLY|O_BINARY,0))<0) ||
-		((oldsize=lseek(fd,0,SEEK_END))==-1) ||
-		((old=malloc(oldsize+1))==NULL) ||
-		(lseek(fd,0,SEEK_SET)!=0) ||
-		(read(fd,old,oldsize)!=oldsize) ||
-		(close(fd)==-1)) err(1,"%s",argv[1]);
-	if((new=malloc(newsize+1))==NULL) err(1,NULL);
-
-	oldpos=0;newpos=0;
-	while(newpos<newsize) {
+	if ((oldsize = lseek(oldfd, 0, SEEK_END)) == -1 ||
+	    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)) == NULL)
+		err(1, NULL);
+
+	oldpos = 0;
+	newpos = 0;
+	while (newpos < newsize) {
 		/* Read control data */
-		for(i=0;i<=2;i++) {
+		for (i = 0; i <= 2; i++) {
 			lenread = BZ2_bzRead(&cbz2err, cpfbz2, buf, 8);
 			if ((lenread < 8) || ((cbz2err != BZ_OK) &&
 			    (cbz2err != BZ_STREAM_END)))
-				errx(1, "Corrupt patch\n");
-			ctrl[i]=offtin(buf);
+				errx(1, "Corrupt patch");
+			ctrl[i] = offtin(buf);
 		}
 
 		/* Sanity-check */
-		if ((ctrl[0] < 0) || (ctrl[1] < 0))
-			errx(1,"Corrupt patch\n");
+		if (ctrl[0] < 0 || ctrl[0] > INT_MAX ||
+		    ctrl[1] < 0 || ctrl[1] > INT_MAX)
+			errx(1, "Corrupt patch");
 
 		/* Sanity-check */
-		if(newpos+ctrl[0]>newsize)
-			errx(1,"Corrupt patch\n");
+		if (newpos + ctrl[0] > newsize)
+			errx(1, "Corrupt patch");
 
 		/* Read diff string */
 		lenread = BZ2_bzRead(&dbz2err, dpfbz2, new + newpos, ctrl[0]);
 		if ((lenread < ctrl[0]) ||
 		    ((dbz2err != BZ_OK) && (dbz2err != BZ_STREAM_END)))
-			errx(1, "Corrupt patch\n");
+			errx(1, "Corrupt patch");
 
 		/* Add old data to diff string */
-		for(i=0;i<ctrl[0];i++)
-			if((oldpos+i>=0) && (oldpos+i<oldsize))
-				new[newpos+i]+=old[oldpos+i];
+		for (i = 0; i < ctrl[0]; i++)
+			if ((oldpos + i >= 0) && (oldpos + i < oldsize))
+				new[newpos + i] += old[oldpos + i];
 
 		/* Adjust pointers */
-		newpos+=ctrl[0];
-		oldpos+=ctrl[0];
+		newpos += ctrl[0];
+		oldpos += ctrl[0];
 
 		/* Sanity-check */
-		if(newpos+ctrl[1]>newsize)
-			errx(1,"Corrupt patch\n");
+		if (newpos + ctrl[1] > newsize)
+			errx(1, "Corrupt patch");
 
 		/* Read extra string */
 		lenread = BZ2_bzRead(&ebz2err, epfbz2, new + newpos, ctrl[1]);
 		if ((lenread < ctrl[1]) ||
 		    ((ebz2err != BZ_OK) && (ebz2err != BZ_STREAM_END)))
-			errx(1, "Corrupt patch\n");
+			errx(1, "Corrupt patch");
 
 		/* Adjust pointers */
 		newpos+=ctrl[1];
@@ -209,12 +286,13 @@ int main(int argc,char * argv[])
 		err(1, "fclose(%s)", argv[3]);
 
 	/* Write the new file */
-	if(((fd=open(argv[2],O_CREAT|O_TRUNC|O_WRONLY|O_BINARY,0666))<0) ||
-		(write(fd,new,newsize)!=newsize) || (close(fd)==-1))
-		err(1,"%s",argv[2]);
+	if (write(newfd, new, newsize) != newsize || close(newfd) == -1)
+		err(1, "%s", argv[2]);
+	/* Disable atexit cleanup */
+	newfile = NULL;
 
 	free(new);
 	free(old);
 
-	return 0;
+	return (0);
 }



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201609222105.u8ML5LLe093825>