[OE-core] [PATCH] openssl: avoid NULL pointer dereference in three places
Saul Wold
sgw at linux.intel.com
Tue Aug 20 19:16:35 UTC 2013
On 08/19/2013 10:07 PM, Xufeng Zhang wrote:
> On 08/20/2013 12:13 PM, Saul Wold wrote:
>> On 08/19/2013 06:59 PM, Xufeng Zhang wrote:
>>> Hi All,
>>>
>>> Anybody help me review this?
>>>
>>>
>>> Thanks,
>>> Xufeng
>>>
>>> On 06/04/2013 02:15 PM, Xufeng Zhang wrote:
>>>> There are three potential NULL pointer dereference in
>>>> EVP_DigestInit_ex(), dh_pub_encode() and dsa_pub_encode()
>>>> functions.
>>>> Fix them by adding proper null pointer check.
>>>>
>>>> [YOCTO #4600]
>>>> [ CQID: WIND00373257 ]
>>>>
>>>> Signed-off-by: Xufeng Zhang<xufeng.zhang at windriver.com>
>>>> ---
>>>> ...-pointer-dereference-in-EVP_DigestInit_ex.patch | 16 +++++++++
>>>> ...NULL-pointer-dereference-in-dh_pub_encode.patch | 34
>>>> ++++++++++++++++++++
>>>> meta/recipes-connectivity/openssl/openssl.inc | 2 +-
>>>> .../recipes-connectivity/openssl/openssl_1.0.1e.bb | 2 +
>>>> 4 files changed, 53 insertions(+), 1 deletions(-)
>>>> create mode 100644
>>>> meta/recipes-connectivity/openssl/openssl-1.0.1e/openssl-avoid-NULL-pointer-dereference-in-EVP_DigestInit_ex.patch
>>>>
>>>>
>>>> create mode 100644
>>>> meta/recipes-connectivity/openssl/openssl-1.0.1e/openssl-avoid-NULL-pointer-dereference-in-dh_pub_encode.patch
>>>>
>>>>
>>>>
>>>> diff --git
>>>> a/meta/recipes-connectivity/openssl/openssl-1.0.1e/openssl-avoid-NULL-pointer-dereference-in-EVP_DigestInit_ex.patch
>>>>
>>>> b/meta/recipes-connectivity/openssl/openssl-1.0.1e/openssl-avoid-NULL-pointer-dereference-in-EVP_DigestInit_ex.patch
>>>>
>>>>
>>>> new file mode 100644
>>>> index 0000000..69924a4
>>>> --- /dev/null
>>>> +++
>>>> b/meta/recipes-connectivity/openssl/openssl-1.0.1e/openssl-avoid-NULL-pointer-dereference-in-EVP_DigestInit_ex.patch
>>>>
>>>>
>>>> @@ -0,0 +1,16 @@
>>>> +openssl: avoid NULL pointer dereference in EVP_DigestInit_ex()
>>>> +
>>>> +We should avoid accessing the type pointer if it's NULL,
>>>> +this could happen if ctx->digest is not NULL.
>>
>> These patches both need Upstream-Status: and Signed-off-by: Tags
>
> Ok, I'll add these stuff when I send V2.
>
Ok, looking at more details, these are good patches and should be
upstreamed also.
Sau!
>>
>>>> +---
>>>> +--- a/crypto/evp/digest.c
>>>> ++++ b/crypto/evp/digest.c
>>>> +@@ -199,7 +199,7 @@
>>>> + return 0;
>>>> + }
>>>> + #endif
>>>> +- if (ctx->digest != type)
>>>> ++ if (type&& (ctx->digest != type))
>>>> + {
>>>> + if (ctx->digest&& ctx->digest->ctx_size)
>>>> + OPENSSL_free(ctx->md_data);
>>
>> I am more concerned about this patch as I do not know the code well,
>> but if ctx->digest is non-NULL and type is, this code would not be
>> executed which would be wrong, correct?
>
> No, that's correct, if type is NULL, we should not execute the below
> code as type pointer is accessed in the following code, see:
> int EVP_DigestInit_ex()
> {
> ...
> if (ctx->digest != type)
> {
> if (ctx->digest && ctx->digest->ctx_size)
> OPENSSL_free(ctx->md_data);
> ctx->digest=type;
> if (!(ctx->flags & EVP_MD_CTX_FLAG_NO_INIT) && type->ctx_size)
> {
> ctx->update = type->update;
> ...
> }
> ...
> }
>
>
>> Your changing the behavior of the code here.
>
> But if we don't change, NULL pointer dereference would appear.
>
>> Also reading the code, it should be be possible for type to be NULL,
> I think you are saying "it should not be possible"
>
>> unless some memory corruption occurs earlier as there is a check at
>> line 153 for !type, which should cause it to goto skip_to_init.
>
> Yes, it's true, but such code is executed only when OPENSSL_NO_ENGINE is
> not defined.
>
>>
>> Please verify this patch is really needed.
>>
>>
>>>> diff --git
>>>> a/meta/recipes-connectivity/openssl/openssl-1.0.1e/openssl-avoid-NULL-pointer-dereference-in-dh_pub_encode.patch
>>>>
>>>> b/meta/recipes-connectivity/openssl/openssl-1.0.1e/openssl-avoid-NULL-pointer-dereference-in-dh_pub_encode.patch
>>>>
>>>>
>>>> new file mode 100644
>>>> index 0000000..642b0ea
>>>> --- /dev/null
>>>> +++
>>>> b/meta/recipes-connectivity/openssl/openssl-1.0.1e/openssl-avoid-NULL-pointer-dereference-in-dh_pub_encode.patch
>>>>
>>>>
>>>> @@ -0,0 +1,34 @@
>>>> +openssl: avoid NULL pointer dereference in
>>>> dh_pub_encode()/dsa_pub_encode()
>>>> +
>>>> +We should avoid accessing the pointer if ASN1_STRING_new()
>>>> +allocates memory failed.
>>>> +---
>>>> +--- a/crypto/dh/dh_ameth.c
>>>> ++++ b/crypto/dh/dh_ameth.c
>>>> +@@ -139,6 +139,12 @@
>>>> + dh=pkey->pkey.dh;
>>>> +
>>>> + str = ASN1_STRING_new();
>>>> ++ if (!str)
>>>> ++ {
>>>> ++ DHerr(DH_F_DH_PUB_ENCODE, ERR_R_MALLOC_FAILURE);
>>>> ++ goto err;
>>>> ++ }
>>>> ++
>>>> + str->length = i2d_DHparams(dh,&str->data);
>>>> + if (str->length<= 0)
>>>> + {
>>>> +--- a/crypto/dsa/dsa_ameth.c
>>>> ++++ b/crypto/dsa/dsa_ameth.c
>>>> +@@ -148,6 +148,11 @@
>>>> + {
>>>> + ASN1_STRING *str;
>>>> + str = ASN1_STRING_new();
>>>> ++ if (!str)
>>>> ++ {
>>>> ++ DSAerr(DSA_F_DSA_PUB_ENCODE, ERR_R_MALLOC_FAILURE);
>>>> ++ goto err;
>>>> ++ }
>>>> + str->length = i2d_DSAparams(dsa,&str->data);
>>>> + if (str->length<= 0)
>>>> + {
>>>> diff --git a/meta/recipes-connectivity/openssl/openssl.inc
>>>> b/meta/recipes-connectivity/openssl/openssl.inc
>>>> index f5b2432..c753a27 100644
>>>> --- a/meta/recipes-connectivity/openssl/openssl.inc
>>>> +++ b/meta/recipes-connectivity/openssl/openssl.inc
>>>> @@ -5,7 +5,7 @@ BUGTRACKER =
>>>> "http://www.openssl.org/news/vulnerabilities.html"
>>>> SECTION = "libs/network"
>>>>
>>>> # Big Jump for OpenSSL 1.0 support with meta-oe
>>>> -INC_PR = "r15"
>>>> +INC_PR = "r16"
>>>>
>> No PR or INC_PR Bump needed
>
> Ok, will drop it.
>
>
> Thanks,
> Xufeng
>
>>
>>>> # "openssl | SSLeay" dual license
>>>> LICENSE = "openssl"
>>>> diff --git a/meta/recipes-connectivity/openssl/openssl_1.0.1e.bb
>>>> b/meta/recipes-connectivity/openssl/openssl_1.0.1e.bb
>>>> index 61de3a6..afd5576 100644
>>>> --- a/meta/recipes-connectivity/openssl/openssl_1.0.1e.bb
>>>> +++ b/meta/recipes-connectivity/openssl/openssl_1.0.1e.bb
>>>> @@ -31,6 +31,8 @@ SRC_URI += "file://configure-targets.patch \
>>>> file://openssl_fix_for_x32.patch \
>>>> file://openssl-fix-doc.patch \
>>>> file://find.pl \
>>>> +
>>>> file://openssl-avoid-NULL-pointer-dereference-in-dh_pub_encode.patch \
>>>> +
>>>> file://openssl-avoid-NULL-pointer-dereference-in-EVP_DigestInit_ex.patch
>>>>
>>>> \
>>>> "
>>>>
>>>> SRC_URI[md5sum] = "66bf6f10f060d561929de96f9dfe5b8c"
>>>
>>> _______________________________________________
>>> Openembedded-core mailing list
>>> Openembedded-core at lists.openembedded.org
>>> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>>>
>>>
>>
>
>
>
More information about the Openembedded-core
mailing list