tests: Fix coverity warnings

Assorted minor code issues, which we should fix so any new errors stand
out more.

BUG=chromium:643769
BRANCH=none
TEST=make runtests

Change-Id: I927571f8a30794c70228506afe4da3eda86f765b
Signed-off-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/383953
Reviewed-by: Daisuke Nojiri <dnojiri@chromium.org>

15 files changed, 102 insertions, 50 deletions

diff --git a/firmware/2lib/include/2return_codes.h b/firmware/2lib/include/2return_codes.h
index af867267..27b80d16 100644
--- a/firmware/2lib/include/2return_codes.h
+++ b/firmware/2lib/include/2return_codes.h
@@ -755,6 +755,14 @@ enum vb2_return_code {
 	VB2_FW_PREAMBLE_CREATE_SIGN,
 
         /**********************************************************************
+	 * Errors generated by unit test functions
+	 */
+	VB2_ERROR_UNIT_TEST = VB2_ERROR_HOST_BASE + 0x060000,
+
+	/* Unable to open an input file needed for a unit test */
+	VB2_ERROR_TEST_INPUT_FILE,
+
+        /**********************************************************************
 	 * Highest non-zero error generated inside vboot library.  Note that
 	 * error codes passed through vboot when it calls external APIs may
 	 * still be outside this range.
diff --git a/tests/bdb_sprw_test.c b/tests/bdb_sprw_test.c
index ccd36624..f4750dae 100644
--- a/tests/bdb_sprw_test.c
+++ b/tests/bdb_sprw_test.c
@@ -76,13 +76,13 @@ static struct bdb_header *create_bdb(const char *key_dir,
 	uint8_t bdbkey_digest[BDB_SHA256_DIGEST_SIZE];
 
 	/* Load keys */
-	sprintf(filename, "%s/bdbkey.keyb", key_dir);
+	snprintf(filename, sizeof(filename), "%s/bdbkey.keyb", key_dir);
 	p.bdbkey = bdb_create_key(filename, 100, "BDB key");
-	sprintf(filename, "%s/datakey.keyb", key_dir);
+	snprintf(filename, sizeof(filename), "%s/datakey.keyb", key_dir);
 	p.datakey = bdb_create_key(filename, 200, "datakey");
-	sprintf(filename, "%s/bdbkey.pem", key_dir);
+	snprintf(filename, sizeof(filename), "%s/bdbkey.pem", key_dir);
 	p.private_bdbkey = read_pem(filename);
-	sprintf(filename, "%s/datakey.pem", key_dir);
+	snprintf(filename, sizeof(filename), "%s/datakey.pem", key_dir);
 	p.private_datakey = read_pem(filename);
 	if (!p.bdbkey || !p.datakey || !p.private_bdbkey || !p.private_datakey) {
 		fprintf(stderr, "Unable to load test keys\n");
diff --git a/tests/bdb_test.c b/tests/bdb_test.c
index da723662..2e8499a4 100644
--- a/tests/bdb_test.c
+++ b/tests/bdb_test.c
@@ -292,13 +292,13 @@ void check_bdb_verify(const char *key_dir)
 	size_t hsize;
 
 	/* Load keys */
-	sprintf(filename, "%s/bdbkey.keyb", key_dir);
+	snprintf(filename, sizeof(filename), "%s/bdbkey.keyb", key_dir);
 	p.bdbkey = bdb_create_key(filename, 100, "BDB key");
-	sprintf(filename, "%s/datakey.keyb", key_dir);
+	snprintf(filename, sizeof(filename), "%s/datakey.keyb", key_dir);
 	p.datakey = bdb_create_key(filename, 200, "datakey");
-	sprintf(filename, "%s/bdbkey.pem", key_dir);
+	snprintf(filename, sizeof(filename), "%s/bdbkey.pem", key_dir);
 	p.private_bdbkey = read_pem(filename);
-	sprintf(filename, "%s/datakey.pem", key_dir);
+	snprintf(filename, sizeof(filename), "%s/datakey.pem", key_dir);
 	p.private_datakey = read_pem(filename);
 	if (!p.bdbkey || !p.datakey || !p.private_bdbkey || !p.private_datakey) {
 		fprintf(stderr, "Unable to load test keys\n");
diff --git a/tests/cgptlib_test.c b/tests/cgptlib_test.c
index f736fd9a..65022a20 100644
--- a/tests/cgptlib_test.c
+++ b/tests/cgptlib_test.c
@@ -173,7 +173,7 @@ static void BuildTestGptData(GptData *gpt)
 	header = (GptHeader *)gpt->primary_header;
 	entries = (GptEntry *)gpt->primary_entries;
 	Memcpy(header->signature, GPT_HEADER_SIGNATURE,
-	       sizeof(GPT_HEADER_SIGNATURE));
+	       GPT_HEADER_SIGNATURE_SIZE);
 	header->revision = GPT_HEADER_REVISION;
 	header->size = sizeof(GptHeader);
 	header->reserved_zero = 0;
diff --git a/tests/rsa_utility_tests.c b/tests/rsa_utility_tests.c
index 7beeed68..0ac3f5e7 100644
--- a/tests/rsa_utility_tests.c
+++ b/tests/rsa_utility_tests.c
@@ -79,7 +79,7 @@ static void TestKeyFromBuffer(void) {
 
   for (i = 0; i < 4; i++) {
     uint32_t key_len = RSA1024NUMBYTES << i;
-    Memset(buf, 0xAB, sizeof(buf));
+    Memset(buf, 0xAB, 8 + 2 * RSA8192NUMBYTES);
     *buf_key_len = key_len / sizeof(uint32_t);
     *(buf_key_len + 1) = 0xF00D2345;  /* n0inv */
     buf[8] = 100;
diff --git a/tests/vb20_common2_tests.c b/tests/vb20_common2_tests.c
index da5b2028..671fa9fe 100644
--- a/tests/vb20_common2_tests.c
+++ b/tests/vb20_common2_tests.c
@@ -142,22 +142,26 @@ int test_algorithm(int key_algorithm, const char *keys_dir)
 
 	struct vb2_private_key *private_key = NULL;
 	struct vb2_signature *sig = NULL;
-	struct vb2_packed_key *key1;
+	struct vb2_packed_key *key1 = NULL;
+
+	int retval = 1;
 
 	printf("***Testing algorithm: %s\n", algo_strings[key_algorithm]);
 
-	sprintf(filename, "%s/key_rsa%d.pem", keys_dir, rsa_len);
+	snprintf(filename, sizeof(filename),
+		 "%s/key_rsa%d.pem", keys_dir, rsa_len);
 	private_key = vb2_read_private_key_pem(filename, key_algorithm);
 	if (!private_key) {
 		fprintf(stderr, "Error reading private_key: %s\n", filename);
-		return 1;
+		goto cleanup_algorithm;
 	}
 
-	sprintf(filename, "%s/key_rsa%d.keyb", keys_dir, rsa_len);
+	snprintf(filename, sizeof(filename),
+		 "%s/key_rsa%d.keyb", keys_dir, rsa_len);
 	key1 = vb2_read_packed_keyb(filename, key_algorithm, 1);
 	if (!key1) {
 		fprintf(stderr, "Error reading public_key: %s\n", filename);
-		return 1;
+		goto cleanup_algorithm;
 	}
 
 	/* Calculate good signatures */
@@ -165,16 +169,22 @@ int test_algorithm(int key_algorithm, const char *keys_dir)
 				      private_key);
 	TEST_PTR_NEQ(sig, 0, "Calculate signature");
 	if (!sig)
-		return 1;
+		goto cleanup_algorithm;
 
 	test_unpack_key(key1);
 	test_verify_data(key1, sig);
 
-	free(key1);
-	free(private_key);
-	free(sig);
+	retval = 0;
+
+cleanup_algorithm:
+	if (key1)
+		free(key1);
+	if (private_key)
+		free(private_key);
+	if (sig)
+		free(sig);
 
-	return 0;
+	return retval;
 }
 
 /* Test only the algorithms we use */
diff --git a/tests/vb20_common3_tests.c b/tests/vb20_common3_tests.c
index 62a36e45..fd15892a 100644
--- a/tests/vb20_common3_tests.c
+++ b/tests/vb20_common3_tests.c
@@ -327,6 +327,7 @@ static void test_verify_fw_preamble(struct vb2_packed_key *public_key,
 
 	free(h);
 	free(hdr);
+	free(body_sig);
 }
 
 static void resign_kernel_preamble(struct vb2_kernel_preamble *h,
@@ -501,6 +502,7 @@ static void test_verify_kernel_preamble(
 
 	free(h);
 	free(hdr);
+	free(body_sig);
 }
 
 int test_permutation(int signing_key_algorithm, int data_key_algorithm,
@@ -509,37 +511,45 @@ int test_permutation(int signing_key_algorithm, int data_key_algorithm,
 	char filename[1024];
 	int signing_rsa_len = siglen_map[signing_key_algorithm] * 8;
 	int data_rsa_len = siglen_map[data_key_algorithm] * 8;
+	int retval = 1;
+
+	struct vb2_private_key *signing_private_key = NULL;
+	struct vb2_packed_key *signing_public_key = NULL;
+	struct vb2_packed_key *data_public_key = NULL;
 
 	printf("***Testing signing algorithm: %s\n",
 	       algo_strings[signing_key_algorithm]);
 	printf("***With data key algorithm: %s\n",
 	       algo_strings[data_key_algorithm]);
 
-	sprintf(filename, "%s/key_rsa%d.pem", keys_dir, signing_rsa_len);
-	struct vb2_private_key *signing_private_key =
+	snprintf(filename, sizeof(filename),
+		 "%s/key_rsa%d.pem", keys_dir, signing_rsa_len);
+	signing_private_key =
 		vb2_read_private_key_pem(filename, signing_key_algorithm);
 	if (!signing_private_key) {
 		fprintf(stderr, "Error reading signing_private_key: %s\n",
 			filename);
-		return 1;
+		goto cleanup_permutation;
 	}
 
-	sprintf(filename, "%s/key_rsa%d.keyb", keys_dir, signing_rsa_len);
-	struct vb2_packed_key *signing_public_key =
+	snprintf(filename, sizeof(filename),
+		 "%s/key_rsa%d.keyb", keys_dir, signing_rsa_len);
+	signing_public_key =
 		vb2_read_packed_keyb(filename, signing_key_algorithm, 1);
 	if (!signing_public_key) {
 		fprintf(stderr, "Error reading signing_public_key: %s\n",
 			filename);
-		return 1;
+		goto cleanup_permutation;
 	}
 
-	sprintf(filename, "%s/key_rsa%d.keyb", keys_dir, data_rsa_len);
-	struct vb2_packed_key *data_public_key =
+	snprintf(filename, sizeof(filename),
+		 "%s/key_rsa%d.keyb", keys_dir, data_rsa_len);
+	data_public_key =
 		vb2_read_packed_keyb(filename, data_key_algorithm, 1);
 	if (!data_public_key) {
 		fprintf(stderr, "Error reading data_public_key: %s\n",
 			filename);
-		return 1;
+		goto cleanup_permutation;
 	}
 
 	/* Unpack public key */
@@ -551,7 +561,7 @@ int test_permutation(int signing_key_algorithm, int data_key_algorithm,
 			   signing_public_key->key_size)) {
 		fprintf(stderr, "Error unpacking signing_public_key: %s\n",
 			filename);
-		return 1;
+		goto cleanup_permutation;
 	}
 
 	test_check_keyblock(&signing_public_key2, signing_private_key,
@@ -562,6 +572,9 @@ int test_permutation(int signing_key_algorithm, int data_key_algorithm,
 				data_public_key);
 	test_verify_kernel_preamble(signing_public_key, signing_private_key);
 
+	retval = 0;
+
+cleanup_permutation:
 	if (signing_public_key)
 		free(signing_public_key);
 	if (signing_private_key)
@@ -569,7 +582,7 @@ int test_permutation(int signing_key_algorithm, int data_key_algorithm,
 	if (data_public_key)
 		free(data_public_key);
 
-	return 0;
+	return retval;
 }
 
 struct test_perm
diff --git a/tests/vb20_verify_fw.c b/tests/vb20_verify_fw.c
index e070ff39..5025859c 100644
--- a/tests/vb20_verify_fw.c
+++ b/tests/vb20_verify_fw.c
@@ -96,11 +96,15 @@ static int hash_body(struct vb2_context *ctx)
 
 	/* Open the body data */
 	f = fopen(body_fname, "rb");
+	if (!f)
+		return VB2_ERROR_TEST_INPUT_FILE;
 
 	/* Start the body hash */
 	rv = vb2api_init_hash(ctx, VB2_HASH_TAG_FW_BODY, &expect_size);
-	if (rv)
+	if (rv) {
+		fclose(f);
 		return rv;
+	}
 
 	printf("Expect %d bytes of body...\n", expect_size);
 
@@ -117,12 +121,16 @@ static int hash_body(struct vb2_context *ctx)
 
 		/* Hash it */
 		rv = vb2api_extend_hash(ctx, block, size);
-		if (rv)
+		if (rv) {
+			fclose(f);
 			return rv;
+		}
 
 		expect_size -= size;
 	}
 
+	fclose(f);
+
 	/* Check the result */
 	rv = vb2api_check_hash(ctx);
 	if (rv)
diff --git a/tests/vb21_common2_tests.c b/tests/vb21_common2_tests.c
index e4e1e2fc..15e4d505 100644
--- a/tests/vb21_common2_tests.c
+++ b/tests/vb21_common2_tests.c
@@ -252,14 +252,16 @@ int test_algorithm(int key_algorithm, const char *keys_dir)
 
 	printf("***Testing algorithm: %s\n", algo_strings[key_algorithm]);
 
-	sprintf(filename, "%s/key_rsa%d.pem", keys_dir, rsa_len);
+	snprintf(filename, sizeof(filename),
+		 "%s/key_rsa%d.pem", keys_dir, rsa_len);
 	TEST_SUCC(vb2_private_key_read_pem(&prik, filename),
 		  "Read private key");
 	prik->hash_alg = hash_alg;
 	prik->sig_alg = sig_alg;
 	vb2_private_key_set_desc(prik, "private key");
 
-	sprintf(filename, "%s/key_rsa%d.keyb", keys_dir, rsa_len);
+	snprintf(filename, sizeof(filename),
+		 "%s/key_rsa%d.keyb", keys_dir, rsa_len);
 	TEST_SUCC(vb2_public_key_read_keyb(&pubk, filename),
 		  "Read public key");
 	pubk->hash_alg = hash_alg;
diff --git a/tests/vb21_host_fw_preamble_tests.c b/tests/vb21_host_fw_preamble_tests.c
index 33b9371a..f563f6ce 100644
--- a/tests/vb21_host_fw_preamble_tests.c
+++ b/tests/vb21_host_fw_preamble_tests.c
@@ -46,13 +46,13 @@ static void preamble_tests(const char *keys_dir)
 	vb2_workbuf_init(&wb, workbuf, sizeof(workbuf));
 
 	/* Read keys */
-	sprintf(fname, "%s/key_rsa4096.keyb", keys_dir);
+	snprintf(fname, sizeof(fname), "%s/key_rsa4096.keyb", keys_dir);
 	TEST_SUCC(vb2_public_key_read_keyb(&pubk4096, fname),
 					   "Read public key 1");
 	vb2_public_key_set_desc(pubk4096, "Test RSA4096 public key");
 	pubk4096->hash_alg = VB2_HASH_SHA256;
 
-	sprintf(fname, "%s/key_rsa4096.pem", keys_dir);
+	snprintf(fname, sizeof(fname), "%s/key_rsa4096.pem", keys_dir);
 	TEST_SUCC(vb2_private_key_read_pem(&prik4096, fname),
 		  "Read private key 2");
 	vb2_private_key_set_desc(prik4096, "Test RSA4096 private key");
diff --git a/tests/vb21_host_key_tests.c b/tests/vb21_host_key_tests.c
index 0e16b048..a833a003 100644
--- a/tests/vb21_host_key_tests.c
+++ b/tests/vb21_host_key_tests.c
@@ -37,8 +37,8 @@ static void private_key_tests(const struct alg_combo *combo,
 	const struct vb2_private_key *ckey;
 	struct vb21_packed_private_key *pkey;
 	char *testfile;
-	const char *notapem = "not_a_pem";
-	const char *testdesc = "test desc";
+	const char notapem[] = "not_a_pem";
+	const char testdesc[] = "test desc";
 	const struct vb2_id test_id = {.raw = {0xaa}};
 	uint8_t *buf, *buf2;
 	uint32_t bufsize;
diff --git a/tests/vb21_host_keyblock_tests.c b/tests/vb21_host_keyblock_tests.c
index 0215b16f..5c4ad0e3 100644
--- a/tests/vb21_host_keyblock_tests.c
+++ b/tests/vb21_host_keyblock_tests.c
@@ -109,11 +109,13 @@ static void keyblock_tests(const char *keys_dir)
 	TEST_EQ(vb21_keyblock_create(&kb, pubk4096, prik, 1, 0, NULL),
 		VB2_KEYBLOCK_CREATE_SIG_SIZE, "Keyblock bad sig size");
 	TEST_PTR_EQ(kb, NULL, "  kb_ptr");
+	free(kb);
 
 	prik[0] = prik4096;
 	pubk4096->sig_alg = VB2_SIG_INVALID;
 	TEST_EQ(vb21_keyblock_create(&kb, pubk4096, prik, 1, 0, NULL),
 		VB2_KEYBLOCK_CREATE_DATA_KEY, "Keyblock bad data key");
+	free(kb);
 
 	/* Free keys */
 	free(pakgood);
diff --git a/tests/vb21_host_sig_tests.c b/tests/vb21_host_sig_tests.c
index ba0ee88d..f4ca1a67 100644
--- a/tests/vb21_host_sig_tests.c
+++ b/tests/vb21_host_sig_tests.c
@@ -171,8 +171,10 @@ static int test_algorithm(const struct alg_combo *combo, const char *keys_dir)
 
 	printf("***Testing algorithm: %s\n", combo->name);
 
-	sprintf(pemfile, "%s/key_rsa%d.pem", keys_dir, rsa_bits);
-	sprintf(keybfile, "%s/key_rsa%d.keyb", keys_dir, rsa_bits);
+	snprintf(pemfile, sizeof(pemfile),
+		 "%s/key_rsa%d.pem", keys_dir, rsa_bits);
+	snprintf(keybfile, sizeof(keybfile),
+		 "%s/key_rsa%d.keyb", keys_dir, rsa_bits);
 
 	sig_tests(combo, pemfile, keybfile);
 
diff --git a/tests/vboot_common2_tests.c b/tests/vboot_common2_tests.c
index 981b4286..adbb2b14 100644
--- a/tests/vboot_common2_tests.c
+++ b/tests/vboot_common2_tests.c
@@ -42,6 +42,8 @@ static void VerifyPublicKeyToRSA(const VbPublicKey *orig_key)
 			"PublicKeyToRSA() algorithm");
 		RSAPublicKeyFree(rsa);
 	}
+
+	free(key);
 }
 
 static void VerifyDataTest(const VbPublicKey *public_key,
@@ -217,6 +219,7 @@ static void VerifyKernelPreambleTest(const VbPublicKey *public_key,
 	free(h);
 	RSAPublicKeyFree(rsa);
 	free(hdr);
+	free(body_sig);
 }
 
 int test_algorithm(int key_algorithm, const char *keys_dir)
@@ -228,7 +231,8 @@ int test_algorithm(int key_algorithm, const char *keys_dir)
 
 	printf("***Testing algorithm: %s\n", algo_strings[key_algorithm]);
 
-	sprintf(filename, "%s/key_rsa%d.pem", keys_dir, rsa_len);
+	snprintf(filename, sizeof(filename),
+		 "%s/key_rsa%d.pem", keys_dir, rsa_len);
 	struct vb2_private_key *private_key =
 		vb2_read_private_key_pem(filename, key_algorithm);
 	if (!private_key) {
@@ -236,11 +240,13 @@ int test_algorithm(int key_algorithm, const char *keys_dir)
 		return 1;
 	}
 
-	sprintf(filename, "%s/key_rsa%d.keyb", keys_dir, rsa_len);
+	snprintf(filename, sizeof(filename),
+		 "%s/key_rsa%d.keyb", keys_dir, rsa_len);
 	public_key = (VbPublicKey *)vb2_read_packed_keyb(filename,
 							 key_algorithm, 1);
 	if (!public_key) {
 		fprintf(stderr, "Error reading public_key: %s\n", filename);
+		free(private_key);
 		return 1;
 	}
 
@@ -249,10 +255,8 @@ int test_algorithm(int key_algorithm, const char *keys_dir)
 	VerifyDigestTest(public_key, private_key);
 	VerifyKernelPreambleTest(public_key, private_key);
 
-	if (public_key)
-		free(public_key);
-	if (private_key)
-		free(private_key);
+	free(public_key);
+	free(private_key);
 
 	return 0;
 }
diff --git a/tests/vboot_common3_tests.c b/tests/vboot_common3_tests.c
index 130deb05..1365631d 100644
--- a/tests/vboot_common3_tests.c
+++ b/tests/vboot_common3_tests.c
@@ -172,7 +172,8 @@ int test_permutation(int signing_key_algorithm, int data_key_algorithm,
 	printf("***With data key algorithm: %s\n",
 	       algo_strings[data_key_algorithm]);
 
-	sprintf(filename, "%s/key_rsa%d.pem", keys_dir, signing_rsa_len);
+	snprintf(filename, sizeof(filename),
+		 "%s/key_rsa%d.pem", keys_dir, signing_rsa_len);
 	struct vb2_private_key *signing_private_key =
 		vb2_read_private_key_pem(filename, signing_key_algorithm);
 	if (!signing_private_key) {
@@ -181,7 +182,8 @@ int test_permutation(int signing_key_algorithm, int data_key_algorithm,
 		return 1;
 	}
 
-	sprintf(filename, "%s/key_rsa%d.keyb", keys_dir, signing_rsa_len);
+	snprintf(filename, sizeof(filename),
+		 "%s/key_rsa%d.keyb", keys_dir, signing_rsa_len);
 	signing_public_key = (VbPublicKey *)
 		vb2_read_packed_keyb(filename, signing_key_algorithm, 1);
 	if (!signing_public_key) {
@@ -190,7 +192,8 @@ int test_permutation(int signing_key_algorithm, int data_key_algorithm,
 		return 1;
 	}
 
-	sprintf(filename, "%s/key_rsa%d.keyb", keys_dir, data_rsa_len);
+	snprintf(filename, sizeof(filename),
+		 "%s/key_rsa%d.keyb", keys_dir, data_rsa_len);
 	struct vb2_packed_key *data_public_key =
 		vb2_read_packed_keyb(filename, data_key_algorithm, 1);
 	if (!data_public_key) {