From f87aa72d4b49ff5fa2760227a99d9a5d90e557c1 Mon Sep 17 00:00:00 2001 From: Randall Spangler Date: Fri, 9 Sep 2016 10:49:37 -0700 Subject: 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 Reviewed-on: https://chromium-review.googlesource.com/383953 Reviewed-by: Daisuke Nojiri --- firmware/2lib/include/2return_codes.h | 8 ++++++++ tests/bdb_sprw_test.c | 8 ++++---- tests/bdb_test.c | 8 ++++---- tests/cgptlib_test.c | 2 +- tests/rsa_utility_tests.c | 2 +- tests/vb20_common2_tests.c | 30 ++++++++++++++++++++---------- tests/vb20_common3_tests.c | 35 ++++++++++++++++++++++++----------- tests/vb20_verify_fw.c | 12 ++++++++++-- tests/vb21_common2_tests.c | 6 ++++-- tests/vb21_host_fw_preamble_tests.c | 4 ++-- tests/vb21_host_key_tests.c | 4 ++-- tests/vb21_host_keyblock_tests.c | 2 ++ tests/vb21_host_sig_tests.c | 6 ++++-- tests/vboot_common2_tests.c | 16 ++++++++++------ tests/vboot_common3_tests.c | 9 ++++++--- 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 @@ -754,6 +754,14 @@ enum vb2_return_code { /* Unable to sign preamble in vb2_create_fw_preamble() */ 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 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) { -- cgit v1.2.1