diff options
| author | tb <> | 2020-10-09 17:19:35 +0000 |
|---|---|---|
| committer | tb <> | 2020-10-09 17:19:35 +0000 |
| commit | c2d858c793f1c66f3c541ebc3f6d0bf4deb3b7e7 (patch) | |
| tree | cb5721a23a34e4af109398c549587aded00d68d7 | |
| parent | acf6e4c16993f0fa6153452c6141084e0c84afc3 (diff) | |
| download | openbsd-c2d858c793f1c66f3c541ebc3f6d0bf4deb3b7e7.tar.gz openbsd-c2d858c793f1c66f3c541ebc3f6d0bf4deb3b7e7.tar.bz2 openbsd-c2d858c793f1c66f3c541ebc3f6d0bf4deb3b7e7.zip | |
Fix leak or double free with OCSP_request_add0_id()
On success, OCSP_request_add0_id() transfers ownership of cid to
either 'one' or 'req' depending on whether the latter is NULL or
not. On failure, the caller can't tell whether OCSP_ONEREQ_new()
failed (in which case cid needs to be freed) or whether it was a
failure to allocate memory in sk_insert() (in which case cid must
not be freed).
The caller is thus faced with the choice of leaving either a leak
or a potential double free. Fix this by transferring ownership
only at the end of the function.
Found while reviewing an upcoming diff by beck.
ok jsing
| -rw-r--r-- | src/lib/libcrypto/ocsp/ocsp_cl.c | 17 |
1 files changed, 9 insertions, 8 deletions
diff --git a/src/lib/libcrypto/ocsp/ocsp_cl.c b/src/lib/libcrypto/ocsp/ocsp_cl.c index 0ed816cdc3..cb5a2f3d18 100644 --- a/src/lib/libcrypto/ocsp/ocsp_cl.c +++ b/src/lib/libcrypto/ocsp/ocsp_cl.c | |||
| @@ -1,4 +1,4 @@ | |||
| 1 | /* $OpenBSD: ocsp_cl.c,v 1.16 2018/11/25 19:48:43 jmc Exp $ */ | 1 | /* $OpenBSD: ocsp_cl.c,v 1.17 2020/10/09 17:19:35 tb Exp $ */ |
| 2 | /* Written by Tom Titchener <Tom_Titchener@groove.net> for the OpenSSL | 2 | /* Written by Tom Titchener <Tom_Titchener@groove.net> for the OpenSSL |
| 3 | * project. */ | 3 | * project. */ |
| 4 | 4 | ||
| @@ -81,18 +81,19 @@ | |||
| 81 | OCSP_ONEREQ * | 81 | OCSP_ONEREQ * |
| 82 | OCSP_request_add0_id(OCSP_REQUEST *req, OCSP_CERTID *cid) | 82 | OCSP_request_add0_id(OCSP_REQUEST *req, OCSP_CERTID *cid) |
| 83 | { | 83 | { |
| 84 | OCSP_ONEREQ *one = NULL; | 84 | OCSP_ONEREQ *one; |
| 85 | 85 | ||
| 86 | if (!(one = OCSP_ONEREQ_new())) | 86 | if ((one = OCSP_ONEREQ_new()) == NULL) |
| 87 | goto err; | 87 | goto err; |
| 88 | if (one->reqCert) | 88 | if (req != NULL) { |
| 89 | OCSP_CERTID_free(one->reqCert); | 89 | if (!sk_OCSP_ONEREQ_push(req->tbsRequest->requestList, one)) |
| 90 | goto err; | ||
| 91 | } | ||
| 92 | OCSP_CERTID_free(one->reqCert); | ||
| 90 | one->reqCert = cid; | 93 | one->reqCert = cid; |
| 91 | if (req && !sk_OCSP_ONEREQ_push(req->tbsRequest->requestList, one)) | ||
| 92 | goto err; | ||
| 93 | return one; | 94 | return one; |
| 94 | 95 | ||
| 95 | err: | 96 | err: |
| 96 | OCSP_ONEREQ_free(one); | 97 | OCSP_ONEREQ_free(one); |
| 97 | return NULL; | 98 | return NULL; |
| 98 | } | 99 | } |