Openembedded Core Discussions
 help / color / mirror / Atom feed
* [PATCH] openssl: avoid NULL pointer dereference in three places
@ 2013-06-04  6:15 Xufeng Zhang
  2013-06-04  6:18 ` Xufeng Zhang
  2013-08-20  1:59 ` Xufeng Zhang
  0 siblings, 2 replies; 6+ messages in thread
From: Xufeng Zhang @ 2013-06-04  6:15 UTC (permalink / raw)
  To: openembedded-core

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@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.
+---
+--- 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);
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"
 
 # "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"
-- 
1.7.0.2



^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] openssl: avoid NULL pointer dereference in three places
  2013-06-04  6:15 [PATCH] openssl: avoid NULL pointer dereference in three places Xufeng Zhang
@ 2013-06-04  6:18 ` Xufeng Zhang
  2013-08-20  1:59 ` Xufeng Zhang
  1 sibling, 0 replies; 6+ messages in thread
From: Xufeng Zhang @ 2013-06-04  6:18 UTC (permalink / raw)
  To: openembedded-core

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@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.
> +---
> +--- 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);
> 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"
>
>   # "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 \
>    
Looks like I have broken the alignment here, I'll change them when I 
send V2 patch.



Thanks,
Xufeng



>              "
>
>   SRC_URI[md5sum] = "66bf6f10f060d561929de96f9dfe5b8c"
>    



^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] openssl: avoid NULL pointer dereference in three places
  2013-06-04  6:15 [PATCH] openssl: avoid NULL pointer dereference in three places Xufeng Zhang
  2013-06-04  6:18 ` Xufeng Zhang
@ 2013-08-20  1:59 ` Xufeng Zhang
  2013-08-20  4:13   ` Saul Wold
  1 sibling, 1 reply; 6+ messages in thread
From: Xufeng Zhang @ 2013-08-20  1:59 UTC (permalink / raw)
  To: openembedded-core

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@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.
> +---
> +--- 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);
> 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"
>
>   # "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"
>    



^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] openssl: avoid NULL pointer dereference in three places
  2013-08-20  1:59 ` Xufeng Zhang
@ 2013-08-20  4:13   ` Saul Wold
  2013-08-20  5:07     ` Xufeng Zhang
  0 siblings, 1 reply; 6+ messages in thread
From: Saul Wold @ 2013-08-20  4:13 UTC (permalink / raw)
  To: Xufeng Zhang; +Cc: openembedded-core

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@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

>> +---
>> +--- 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?  Your changing the behavior of the code 
here. Also reading the code, it should be be possible for type to be 
NULL, 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.

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

>>   # "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@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>
>


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] openssl: avoid NULL pointer dereference in three places
  2013-08-20  4:13   ` Saul Wold
@ 2013-08-20  5:07     ` Xufeng Zhang
  2013-08-20 19:16       ` Saul Wold
  0 siblings, 1 reply; 6+ messages in thread
From: Xufeng Zhang @ 2013-08-20  5:07 UTC (permalink / raw)
  To: Saul Wold; +Cc: openembedded-core

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@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.

>
>>> +---
>>> +--- 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@lists.openembedded.org
>> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>>
>>
>



^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] openssl: avoid NULL pointer dereference in three places
  2013-08-20  5:07     ` Xufeng Zhang
@ 2013-08-20 19:16       ` Saul Wold
  0 siblings, 0 replies; 6+ messages in thread
From: Saul Wold @ 2013-08-20 19:16 UTC (permalink / raw)
  To: Xufeng Zhang; +Cc: openembedded-core

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@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@lists.openembedded.org
>>> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>>>
>>>
>>
>
>
>


^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-08-20 19:16 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-06-04  6:15 [PATCH] openssl: avoid NULL pointer dereference in three places Xufeng Zhang
2013-06-04  6:18 ` Xufeng Zhang
2013-08-20  1:59 ` Xufeng Zhang
2013-08-20  4:13   ` Saul Wold
2013-08-20  5:07     ` Xufeng Zhang
2013-08-20 19:16       ` Saul Wold

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox