Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

check the return value of EVP_MD_CTX_create(EVP_MD_CTX_new) #8133

Closed
wants to merge 8 commits into from
2 changes: 1 addition & 1 deletion lib/curl_hmac.h
Expand Up @@ -26,7 +26,7 @@

#define HMAC_MD5_LENGTH 16

typedef void (* HMAC_hinit_func)(void *context);
typedef CURLcode (* HMAC_hinit_func)(void *context);
typedef void (* HMAC_hupdate_func)(void *context,
const unsigned char *data,
unsigned int len);
Expand Down
4 changes: 2 additions & 2 deletions lib/curl_md5.h
Expand Up @@ -27,7 +27,7 @@

#define MD5_DIGEST_LEN 16

typedef void (* Curl_MD5_init_func)(void *context);
typedef CURLcode (* Curl_MD5_init_func)(void *context);
typedef void (* Curl_MD5_update_func)(void *context,
const unsigned char *data,
unsigned int len);
Expand All @@ -49,7 +49,7 @@ struct MD5_context {
extern const struct MD5_params Curl_DIGEST_MD5[1];
extern const struct HMAC_params Curl_HMAC_MD5[1];

void Curl_md5it(unsigned char *output, const unsigned char *input,
CURLcode Curl_md5it(unsigned char *output, const unsigned char *input,
const size_t len);

struct MD5_context *Curl_MD5_init(const struct MD5_params *md5params);
Expand Down
2 changes: 1 addition & 1 deletion lib/curl_sha256.h
Expand Up @@ -37,7 +37,7 @@ extern const struct HMAC_params Curl_HMAC_SHA256[1];
#define SHA256_DIGEST_LENGTH 32
#endif

void Curl_sha256it(unsigned char *outbuffer, const unsigned char *input,
CURLcode Curl_sha256it(unsigned char *outbuffer, const unsigned char *input,
const size_t len);

#endif
Expand Down
14 changes: 10 additions & 4 deletions lib/http_aws_sigv4.c
Expand Up @@ -286,8 +286,11 @@ CURLcode Curl_output_aws_sigv4(struct Curl_easy *data, bool proxy)
post_data_len = strlen(post_data);
else
post_data_len = (size_t)data->set.postfieldsize;
Curl_sha256it(sha_hash,
(const unsigned char *) post_data, post_data_len);
if(Curl_sha256it(sha_hash, (const unsigned char *) post_data,
post_data_len) != CURLE_OK) {
goto fail;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please make Curl_sha256it return error/success instead and check that instead of relying on the output data. Yes, that's a bigger change but is much cleaner and nicer code.


sha256_to_hex(sha_hex, sha_hash, sizeof(sha_hex));

Curl_http_method(data, conn, &method, &httpreq);
Expand Down Expand Up @@ -320,8 +323,11 @@ CURLcode Curl_output_aws_sigv4(struct Curl_easy *data, bool proxy)
goto fail;
}

Curl_sha256it(sha_hash, (unsigned char *) canonical_request,
strlen(canonical_request));
if(Curl_sha256it(sha_hash, (unsigned char *) canonical_request,
strlen(canonical_request)) != CURLE_OK) {
goto fail;
}

sha256_to_hex(sha_hex, sha_hash, sizeof(sha_hex));

/*
Expand Down
23 changes: 16 additions & 7 deletions lib/md5.c
Expand Up @@ -62,9 +62,10 @@

typedef struct md5_ctx MD5_CTX;

static void MD5_Init(MD5_CTX *ctx)
static CURLcode MD5_Init(MD5_CTX *ctx)
{
md5_init(ctx);
return CURLE_OK;
}

static void MD5_Update(MD5_CTX *ctx,
Expand Down Expand Up @@ -98,13 +99,14 @@ static void MD5_Final(unsigned char *digest, MD5_CTX *ctx)

typedef mbedtls_md5_context MD5_CTX;

static void MD5_Init(MD5_CTX *ctx)
static CURLcode MD5_Init(MD5_CTX *ctx)
{
#if !defined(HAS_MBEDTLS_RESULT_CODE_BASED_FUNCTIONS)
(void) mbedtls_md5_starts(ctx);
#else
(void) mbedtls_md5_starts_ret(ctx);
#endif
return CURLE_OK;
}

static void MD5_Update(MD5_CTX *ctx,
Expand Down Expand Up @@ -146,9 +148,10 @@ static void MD5_Final(unsigned char *digest, MD5_CTX *ctx)
/* The last #include file should be: */
#include "memdebug.h"

static void MD5_Init(MD5_CTX *ctx)
static CURLcode MD5_Init(MD5_CTX *ctx)
{
CC_MD5_Init(ctx);
return CURLE_OK;
}

static void MD5_Update(MD5_CTX *ctx,
Expand Down Expand Up @@ -176,12 +179,13 @@ struct md5_ctx {
};
typedef struct md5_ctx MD5_CTX;

static void MD5_Init(MD5_CTX *ctx)
static CURLcode MD5_Init(MD5_CTX *ctx)
{
if(CryptAcquireContext(&ctx->hCryptProv, NULL, NULL, PROV_RSA_FULL,
CRYPT_VERIFYCONTEXT | CRYPT_SILENT)) {
CryptCreateHash(ctx->hCryptProv, CALG_MD5, 0, 0, &ctx->hHash);
}
return CURLE_OK;
}

static void MD5_Update(MD5_CTX *ctx,
Expand Down Expand Up @@ -261,7 +265,7 @@ struct md5_ctx {
};
typedef struct md5_ctx MD5_CTX;

static void MD5_Init(MD5_CTX *ctx);
static CURLcode MD5_Init(MD5_CTX *ctx);
static void MD5_Update(MD5_CTX *ctx, const void *data, unsigned long size);
static void MD5_Final(unsigned char *result, MD5_CTX *ctx);

Expand Down Expand Up @@ -422,7 +426,7 @@ static const void *body(MD5_CTX *ctx, const void *data, unsigned long size)
return ptr;
}

static void MD5_Init(MD5_CTX *ctx)
static CURLcode MD5_Init(MD5_CTX *ctx)
{
ctx->a = 0x67452301;
ctx->b = 0xefcdab89;
Expand All @@ -431,6 +435,8 @@ static void MD5_Init(MD5_CTX *ctx)

ctx->lo = 0;
ctx->hi = 0;

return CURLE_OK;
}

static void MD5_Update(MD5_CTX *ctx, const void *data, unsigned long size)
Expand Down Expand Up @@ -555,15 +561,18 @@ const struct MD5_params Curl_DIGEST_MD5[] = {

/*
* @unittest: 1601
* Returns CURLE_OK on success.
*/
void Curl_md5it(unsigned char *outbuffer, const unsigned char *input,
CURLcode Curl_md5it(unsigned char *outbuffer, const unsigned char *input,
const size_t len)
{
MD5_CTX ctx;

MD5_Init(&ctx);
MD5_Update(&ctx, input, curlx_uztoui(len));
MD5_Final(outbuffer, &ctx);

return CURLE_OK;
}

struct MD5_context *Curl_MD5_init(const struct MD5_params *md5params)
Expand Down
38 changes: 28 additions & 10 deletions lib/sha256.c
Expand Up @@ -82,10 +82,14 @@ struct sha256_ctx {
};
typedef struct sha256_ctx my_sha256_ctx;

static void my_sha256_init(my_sha256_ctx *ctx)
static CURLcode my_sha256_init(my_sha256_ctx *ctx)
{
ctx->openssl_ctx = EVP_MD_CTX_create();
if(!ctx->openssl_ctx)
return CURLE_OUT_OF_MEMORY;

EVP_DigestInit_ex(ctx->openssl_ctx, EVP_sha256(), NULL);
return CURLE_OK;
}

static void my_sha256_update(my_sha256_ctx *ctx,
Expand All @@ -112,9 +116,10 @@ static void my_sha256_final(unsigned char *digest, my_sha256_ctx *ctx)

typedef struct sha256_ctx my_sha256_ctx;

static void my_sha256_init(my_sha256_ctx *ctx)
static CURLcode my_sha256_init(my_sha256_ctx *ctx)
{
sha256_init(ctx);
return CURLE_OK;
}

static void my_sha256_update(my_sha256_ctx *ctx,
Expand All @@ -140,13 +145,14 @@ static void my_sha256_final(unsigned char *digest, my_sha256_ctx *ctx)

typedef mbedtls_sha256_context my_sha256_ctx;

static void my_sha256_init(my_sha256_ctx *ctx)
static CURLcode my_sha256_init(my_sha256_ctx *ctx)
{
#if !defined(HAS_MBEDTLS_RESULT_CODE_BASED_FUNCTIONS)
(void) mbedtls_sha256_starts(ctx, 0);
#else
(void) mbedtls_sha256_starts_ret(ctx, 0);
#endif
return CURLE_OK;
}

static void my_sha256_update(my_sha256_ctx *ctx,
Expand Down Expand Up @@ -183,9 +189,10 @@ static void my_sha256_final(unsigned char *digest, my_sha256_ctx *ctx)

typedef CC_SHA256_CTX my_sha256_ctx;

static void my_sha256_init(my_sha256_ctx *ctx)
static CURLcode my_sha256_init(my_sha256_ctx *ctx)
{
(void) CC_SHA256_Init(ctx);
return CURLE_OK;
}

static void my_sha256_update(my_sha256_ctx *ctx,
Expand Down Expand Up @@ -214,12 +221,14 @@ typedef struct sha256_ctx my_sha256_ctx;
#define CALG_SHA_256 0x0000800c
#endif

static void my_sha256_init(my_sha256_ctx *ctx)
static CURLcode my_sha256_init(my_sha256_ctx *ctx)
{
if(CryptAcquireContext(&ctx->hCryptProv, NULL, NULL, PROV_RSA_AES,
CRYPT_VERIFYCONTEXT | CRYPT_SILENT)) {
CryptCreateHash(ctx->hCryptProv, CALG_SHA_256, 0, 0, &ctx->hHash);
}

return CURLE_OK;
}

static void my_sha256_update(my_sha256_ctx *ctx,
Expand Down Expand Up @@ -375,7 +384,7 @@ static int sha256_compress(struct sha256_state *md,
}

/* Initialize the hash state */
static void my_sha256_init(struct sha256_state *md)
static CURLcode my_sha256_init(struct sha256_state *md)
{
md->curlen = 0;
md->length = 0;
Expand All @@ -387,14 +396,16 @@ static void my_sha256_init(struct sha256_state *md)
md->state[5] = 0x9B05688CUL;
md->state[6] = 0x1F83D9ABUL;
md->state[7] = 0x5BE0CD19UL;

return CURLE_OK;
}

/*
Process a block of memory though the hash
@param md The hash state
@param in The data to hash
@param inlen The length of the data (octets)
@return CRYPT_OK if successful
@return 0 if successful
*/
static int my_sha256_update(struct sha256_state *md,
const unsigned char *in,
Expand Down Expand Up @@ -435,7 +446,7 @@ static int my_sha256_update(struct sha256_state *md,
Terminate the hash to get the digest
@param md The hash state
@param out [out] The destination of the hash (32 bytes)
@return CRYPT_OK if successful
@return 0 if successful
*/
static int my_sha256_final(unsigned char *out,
struct sha256_state *md)
Expand Down Expand Up @@ -491,15 +502,22 @@ static int my_sha256_final(unsigned char *out,
* output [in/out] - The output buffer.
* input [in] - The input data.
* length [in] - The input length.
*
* Returns CURLE_OK on success.
*/
void Curl_sha256it(unsigned char *output, const unsigned char *input,
CURLcode Curl_sha256it(unsigned char *output, const unsigned char *input,
const size_t length)
{
CURLcode ret;
my_sha256_ctx ctx;

my_sha256_init(&ctx);
ret = my_sha256_init(&ctx);
if(ret != CURLE_OK)
return ret;

my_sha256_update(&ctx, input, curlx_uztoui(length));
my_sha256_final(output, &ctx);
return ret;
}


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't comment on line 527, but the HMAC_hinit_func signature (and the MD5 init function) would have to be updated too to return CURLcode. Unfortunately the CURLX_FUNCTION_CAST meant for the parameter hides that. Looks good to me otherwise!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I forgot to update it though you reminded me. I will do it after work...

Expand Down
2 changes: 1 addition & 1 deletion lib/vauth/digest.c
Expand Up @@ -666,7 +666,7 @@ static CURLcode auth_create_digest_http_message(
struct digestdata *digest,
char **outptr, size_t *outlen,
void (*convert_to_ascii)(unsigned char *, unsigned char *),
void (*hash)(unsigned char *, const unsigned char *,
CURLcode (*hash)(unsigned char *, const unsigned char *,
const size_t))
{
CURLcode result;
Expand Down
4 changes: 3 additions & 1 deletion lib/vauth/ntlm.c
Expand Up @@ -603,7 +603,9 @@ CURLcode Curl_auth_create_ntlm_type3_message(struct Curl_easy *data,
memcpy(tmp, &ntlm->nonce[0], 8);
memcpy(tmp + 8, entropy, 8);

Curl_md5it(md5sum, tmp, 16);
result = Curl_md5it(md5sum, tmp, 16);
if(result)
return result;

/* We shall only use the first 8 bytes of md5sum, but the des code in
Curl_ntlm_core_lm_resp only encrypt the first 8 bytes */
Expand Down
7 changes: 4 additions & 3 deletions lib/vssh/libssh2.c
Expand Up @@ -645,8 +645,9 @@ static CURLcode ssh_check_fingerprint(struct Curl_easy *data)

hostkey = libssh2_session_hostkey(sshc->ssh_session, &len, NULL);
if(hostkey) {
Curl_sha256it(hash, (const unsigned char *) hostkey, len);
fingerprint = (char *) hash;
if(Curl_sha256it(hash, (const unsigned char *) hostkey,
len) == CURLE_OK)
fingerprint = (char *) hash;
}
#endif

Expand All @@ -661,7 +662,7 @@ static CURLcode ssh_check_fingerprint(struct Curl_easy *data)

/* The length of fingerprint is 32 bytes for SHA256.
* See libssh2_hostkey_hash documentation. */
if(Curl_base64_encode (data, fingerprint, 32, &fingerprint_b64,
if(Curl_base64_encode(data, fingerprint, 32, &fingerprint_b64,
&fingerprint_b64_len) != CURLE_OK) {
state(data, SSH_SESSION_FREE);
sshc->actualcode = CURLE_PEER_FAILED_VERIFICATION;
Expand Down