From owner-svn-src-all@freebsd.org Sat Feb 4 14:10:18 2017 Return-Path: Delivered-To: svn-src-all@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 376A7CCF466; Sat, 4 Feb 2017 14:10:18 +0000 (UTC) (envelope-from def@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 0181EF78; Sat, 4 Feb 2017 14:10:17 +0000 (UTC) (envelope-from def@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id v14EAHBj007172; Sat, 4 Feb 2017 14:10:17 GMT (envelope-from def@FreeBSD.org) Received: (from def@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id v14EAGAn007170; Sat, 4 Feb 2017 14:10:16 GMT (envelope-from def@FreeBSD.org) Message-Id: <201702041410.v14EAGAn007170@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: def set sender to def@FreeBSD.org using -f From: Konrad Witaszczyk Date: Sat, 4 Feb 2017 14:10:16 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r313195 - in head/sbin: decryptcore savecore X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 04 Feb 2017 14:10:18 -0000 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 #include #include -#include #include #include @@ -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); }