svn commit: r313195 - in head/sbin: decryptcore savecore

[Konrad Witaszczyk]

Author: def
Date: Sat Feb  4 14:10:16 2017
New Revision: 313195
URL: https://svnweb.freebsd.org/changeset/base/313195

Log:
  Fix bugs found by Coverity in decryptcore(8) and savecore(8):
  - Perform final decryption and write decrypted data in case of non-block aligned
  input data;
  - Use strlcpy(3) instead of strncpy(3) to verify if paths aren't too long;
  - Check errno after calling unlink(2) instead of calling stat(2) in order to
  verify if a decrypted core was created by a child process;
  - Free dumpkey.
  
  Reported by:	Coverity, cem, pfg
  Suggested by:	cem
  CID:		1366936, 1366942, 1366951, 1366952
  Approved by:	pjd (mentor)

Modified:
  head/sbin/decryptcore/decryptcore.c
  head/sbin/savecore/savecore.c

Modified: head/sbin/decryptcore/decryptcore.c
==============================================================================
--- head/sbin/decryptcore/decryptcore.c	Sat Feb  4 12:26:38 2017	(r313194)
+++ head/sbin/decryptcore/decryptcore.c	Sat Feb  4 14:10:16 2017	(r313195)
@@ -31,7 +31,6 @@ __FBSDID("$FreeBSD$");
 #include <sys/capsicum.h>
 #include <sys/endian.h>
 #include <sys/kerneldump.h>
-#include <sys/stat.h>
 #include <sys/sysctl.h>
 #include <sys/wait.h>
 
@@ -232,8 +231,6 @@ decrypt(const char *privkeyfile, const c
 			pjdlog_errno(LOG_ERR, "Unable to read data from %s",
 			    input);
 			goto failed;
-		} else if (bytes == 0) {
-			break;
 		}
 
 		if (bytes > 0) {
@@ -249,10 +246,7 @@ decrypt(const char *privkeyfile, const c
 			}
 		}
 
-		if (olen == 0)
-			continue;
-
-		if (write(ofd, buf, olen) != olen) {
+		if (olen > 0 && write(ofd, buf, olen) != olen) {
 			pjdlog_errno(LOG_ERR, "Unable to write data to %s",
 			    output);
 			goto failed;
@@ -274,7 +268,6 @@ int
 main(int argc, char **argv)
 {
 	char core[PATH_MAX], encryptedcore[PATH_MAX], keyfile[PATH_MAX];
-	struct stat sb;
 	const char *crashdir, *dumpnr, *privatekey;
 	int ch, debug;
 	size_t ii;
@@ -297,16 +290,23 @@ main(int argc, char **argv)
 			usesyslog = true;
 			break;
 		case 'c':
-			strncpy(core, optarg, sizeof(core));
+			if (strlcpy(core, optarg, sizeof(core)) >= sizeof(core))
+				pjdlog_exitx(1, "Core file path is too long.");
 			break;
 		case 'd':
 			crashdir = optarg;
 			break;
 		case 'e':
-			strncpy(encryptedcore, optarg, sizeof(encryptedcore));
+			if (strlcpy(encryptedcore, optarg,
+			    sizeof(encryptedcore)) >= sizeof(encryptedcore)) {
+				pjdlog_exitx(1, "Encrypted core file path is too long.");
+			}
 			break;
 		case 'k':
-			strncpy(keyfile, optarg, sizeof(keyfile));
+			if (strlcpy(keyfile, optarg, sizeof(keyfile)) >=
+			    sizeof(keyfile)) {
+				pjdlog_exitx(1, "Key file path is too long.");
+			}
 			break;
 		case 'n':
 			dumpnr = optarg;
@@ -362,7 +362,7 @@ main(int argc, char **argv)
 	pjdlog_debug_set(debug);
 
 	if (!decrypt(privatekey, keyfile, encryptedcore, core)) {
-		if (stat(core, &sb) == 0 && unlink(core) != 0)
+		if (unlink(core) == -1 && errno != ENOENT)
 			pjdlog_exit(1, "Unable to remove core");
 		exit(1);
 	}

Modified: head/sbin/savecore/savecore.c
==============================================================================
--- head/sbin/savecore/savecore.c	Sat Feb  4 12:26:38 2017	(r313194)
+++ head/sbin/savecore/savecore.c	Sat Feb  4 14:10:16 2017	(r313195)
@@ -478,6 +478,7 @@ DoFile(const char *savedir, const char *
 	bool isencrypted, ret;
 
 	bounds = getbounds();
+	dumpkey = NULL;
 	mediasize = 0;
 	status = STATUS_UNKNOWN;
 
@@ -816,6 +817,7 @@ nuke:
 	}
 	xo_close_container_h(xostdout, "crashdump");
 	xo_finish_h(xostdout);
+	free(dumpkey);
 	free(temp);
 	close(fd);
 	return;
@@ -824,6 +826,7 @@ closeall:
 	fclose(fp);
 
 closefd:
+	free(dumpkey);
 	free(temp);
 	close(fd);
 }