Commit 62b0a0de authored by Benjamin Kaduk's avatar Benjamin Kaduk Committed by Rich Salz
Browse files

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: default avatarRichard Levitte <levitte@openssl.org>
Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3379)
parent 388d679a

crypto/ct/ct_b64.c

+8 −4
Original line number Diff line number Diff line
@@ -24,7 +24,7 @@ 
static int ct_base64_decode(const char *in, unsigned char **out)

{

    size_t inlen = strlen(in);

    int outlen;

    int outlen, i;

    unsigned char *outbuf = NULL;



    if (inlen == 0) {
@@ -45,9 +45,12 @@ static int ct_base64_decode(const char *in, unsigned char **out) 
        goto err;

    }



    /* Subtract padding bytes from |outlen| */

    /* Subtract padding bytes from |outlen|.  Any more than 2 is malformed. */

    i = 0;

    while (in[--inlen] == '=') {

        --outlen;

        if (++i > 2)

            goto err;

    }



    *out = outbuf;
@@ -132,7 +135,7 @@ SCT *SCT_new_from_base64(unsigned char version, const char *logid_base64, 
int CTLOG_new_from_base64(CTLOG **ct_log, const char *pkey_base64, const char *name)

{

    unsigned char *pkey_der = NULL;

    int pkey_der_len = ct_base64_decode(pkey_base64, &pkey_der);

    int pkey_der_len;

    const unsigned char *p;

    EVP_PKEY *pkey = NULL;
@@ -141,7 +144,8 @@ int CTLOG_new_from_base64(CTLOG **ct_log, const char *pkey_base64, const char *n 
        return 0;

    }



    if (pkey_der_len <= 0) {

    pkey_der_len = ct_base64_decode(pkey_base64, &pkey_der);

    if (pkey_der_len < 0) {

        CTerr(CT_F_CTLOG_NEW_FROM_BASE64, CT_R_LOG_CONF_INVALID_KEY);

        return 0;

    }

test/ct_test.c

+24 −9
Original line number Diff line number Diff line
@@ -344,7 +344,7 @@ end: 
#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 @@ static int test_no_scts_in_certificate() 
    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 @@ static int test_one_sct_in_certificate() 
    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 @@ static int test_multiple_scts_in_certificate() 
    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 @@ static int test_verify_one_sct() 
    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 @@ static int test_verify_multiple_scts() 
    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 @@ static int test_verify_fails_for_future_sct() 
    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 @@ static int test_decode_tls_sct() 
    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 @@ static int test_encode_tls_sct() 
 * 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 @@ end: 
    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 @@ int test_main(int argc, char *argv[]) 
    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]);

}