commit 62b0a0dea612e3683c6bd4bef359fceda00238e8
author Benjamin Kaduk <bkaduk@akamai.com> Fri Jun 09 13:31:11 2017 -0400
committer Rich Salz <rsalz@openssl.org> Fri Jun 09 13:32:29 2017 -0400
tree 46da1e6a25e540698cb6b896440c8f70fffe6d2b
parent 388d679a4fd8a408e7c7c1867cc974cdc977ae63

Fix memory leaks in CTLOG_new_from_base64

Move the call to ct_base64_decode(), which allocates, until after
the check for NULL output parameter.

Also place a cap on the number of padding characters used to decrement
the output length -- any more than two '='s is not permitted in a
well-formed base64 text.  Prior to this change, ct_base64_decode() would
return a length of -1 along with allocated storage for an input of
"====".

Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3379)

test/ct_test.c

diff --git a/test/ct_test.c b/test/ct_test.c
index 6b36a43..b551b85 100644
--- a/test/ct_test.c
+++ b/test/ct_test.c
@@ -344,7 +344,7 @@
 #define SETUP_CT_TEST_FIXTURE() SETUP_TEST_FIXTURE(CT_TEST_FIXTURE, set_up)
 #define EXECUTE_CT_TEST() EXECUTE_TEST(execute_cert_test, tear_down)
 
-static int test_no_scts_in_certificate()
+static int test_no_scts_in_certificate(void)
 {
     SETUP_CT_TEST_FIXTURE();
     fixture.certs_dir = certs_dir;
@@ -354,7 +354,7 @@
     EXECUTE_CT_TEST();
 }
 
-static int test_one_sct_in_certificate()
+static int test_one_sct_in_certificate(void)
 {
     SETUP_CT_TEST_FIXTURE();
     fixture.certs_dir = certs_dir;
@@ -366,7 +366,7 @@
     EXECUTE_CT_TEST();
 }
 
-static int test_multiple_scts_in_certificate()
+static int test_multiple_scts_in_certificate(void)
 {
     SETUP_CT_TEST_FIXTURE();
     fixture.certs_dir = certs_dir;
@@ -378,7 +378,7 @@
     EXECUTE_CT_TEST();
 }
 
-static int test_verify_one_sct()
+static int test_verify_one_sct(void)
 {
     SETUP_CT_TEST_FIXTURE();
     fixture.certs_dir = certs_dir;
@@ -389,7 +389,7 @@
     EXECUTE_CT_TEST();
 }
 
-static int test_verify_multiple_scts()
+static int test_verify_multiple_scts(void)
 {
     SETUP_CT_TEST_FIXTURE();
     fixture.certs_dir = certs_dir;
@@ -400,7 +400,7 @@
     EXECUTE_CT_TEST();
 }
 
-static int test_verify_fails_for_future_sct()
+static int test_verify_fails_for_future_sct(void)
 {
     SETUP_CT_TEST_FIXTURE();
     fixture.epoch_time_in_ms = 1365094800000; /* Apr 4 17:00:00 2013 GMT */
@@ -413,7 +413,7 @@
     EXECUTE_CT_TEST();
 }
 
-static int test_decode_tls_sct()
+static int test_decode_tls_sct(void)
 {
     const unsigned char tls_sct_list[] = "\x00\x78" /* length of list */
         "\x00\x76"
@@ -441,7 +441,7 @@
     EXECUTE_CT_TEST();
 }
 
-static int test_encode_tls_sct()
+static int test_encode_tls_sct(void)
 {
     const char log_id[] = "3xwuwRUAlFJHqWFoMl3cXHlZ6PfG04j8AC4LvT9012Q=";
     const uint64_t timestamp = 1;
@@ -469,7 +469,7 @@
  * Tests that the CT_POLICY_EVAL_CTX default time is approximately now.
  * Allow +-10 minutes, as it may compensate for clock skew.
  */
-static int test_default_ct_policy_eval_ctx_time_is_now()
+static int test_default_ct_policy_eval_ctx_time_is_now(void)
 {
     int success = 0;
     CT_POLICY_EVAL_CTX *ct_policy_ctx = CT_POLICY_EVAL_CTX_new();
@@ -486,6 +486,20 @@
     return success;
 }
 
+static int test_ctlog_from_base64(void)
+{
+    CTLOG *log = NULL;
+    const char notb64[] = "\01\02\03\04";
+    const char pad[] = "====";
+    const char name[] = "name";
+
+    /* We expect these to both fail! */
+    if (!TEST_true(!CTLOG_new_from_base64(&log, notb64, name))
+        || !TEST_true(!CTLOG_new_from_base64(&log, pad, name)))
+        return 0;
+    return 1;
+}
+
 int test_main(int argc, char *argv[])
 {
     if ((ct_dir = getenv("CT_DIR")) == NULL)
@@ -502,6 +516,7 @@
     ADD_TEST(test_decode_tls_sct);
     ADD_TEST(test_encode_tls_sct);
     ADD_TEST(test_default_ct_policy_eval_ctx_time_is_now);
+    ADD_TEST(test_ctlog_from_base64);
 
     return run_tests(argv[0]);
 }