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

BUG: Curl_nss_md5sum/Curl_nss_sha256sum doesn't check context value #5302

Closed
gfphoenix78 opened this issue Apr 27, 2020 · 2 comments
Closed

BUG: Curl_nss_md5sum/Curl_nss_sha256sum doesn't check context value #5302

gfphoenix78 opened this issue Apr 27, 2020 · 2 comments
Labels

Comments

@gfphoenix78
Copy link

@gfphoenix78 gfphoenix78 commented Apr 27, 2020

Curl_nss_md5sum/Curl_nss_sha256sum doesn't check the context pointer, which may be NULL.
Passing NULL pointer to PK11_DigestOp() will cause SIGSEGV if the input data is not empty.

static CURLcode Curl_nss_md5sum(unsigned char *tmp, /* input */
                                size_t tmplen,
                                unsigned char *md5sum, /* output */
                                size_t md5len)
{
  PK11Context *MD5pw = PK11_CreateDigestContext(SEC_OID_MD5);
  unsigned int MD5out;

  PK11_DigestOp(MD5pw, tmp, curlx_uztoui(tmplen));
  PK11_DigestFinal(MD5pw, md5sum, &MD5out, curlx_uztoui(md5len));
  PK11_DestroyContext(MD5pw, PR_TRUE);

  return CURLE_OK;
}

static CURLcode Curl_nss_sha256sum(const unsigned char *tmp, /* input */
                               size_t tmplen,
                               unsigned char *sha256sum, /* output */
                               size_t sha256len)
{
  PK11Context *SHA256pw = PK11_CreateDigestContext(SEC_OID_SHA256);
  unsigned int SHA256out;

  PK11_DigestOp(SHA256pw, tmp, curlx_uztoui(tmplen));
  PK11_DigestFinal(SHA256pw, sha256sum, &SHA256out, curlx_uztoui(sha256len));
  PK11_DestroyContext(SHA256pw, PR_TRUE);

  return CURLE_OK;
}

The following code is from the master branch of nss

SECStatus
PK11_DigestOp(PK11Context *context, const unsigned char *in, unsigned inLen)
{
    CK_RV crv = CKR_OK;
    SECStatus rv = SECSuccess;

    if (inLen == 0) {
        return SECSuccess;
    }
    if (!in) {
        PORT_SetError(SEC_ERROR_INVALID_ARGS);
        return SECFailure;
    }

    /* if we ran out of session, we need to restore our previously stored
     * state.
     */
    context->init = PR_FALSE;
... ...

I expected the following

curl/libcurl version

master branch. Maybe other branches also have this bug.

@bagder
Copy link
Member

@bagder bagder commented Apr 27, 2020

I wish NSS was documented so we could actually read how PK11_CreateDigestContext() is supposed to behave...

@danielgustafsson
Copy link
Member

@danielgustafsson danielgustafsson commented Apr 27, 2020

PK11_CreateDigestContext will return NULL in case there is no slot/module for the requested algorithm.

bagder added a commit that referenced this issue Apr 27, 2020
... to avoid crashes!

Reported-by: Hao Wu
Fixes #5302
@bagder bagder closed this in cad15b9 Apr 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants
You can’t perform that action at this time.