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<[email protected]>
---
...-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
[email protected]
http://lists.openembedded.org/mailman/listinfo/openembedded-core
_______________________________________________
Openembedded-core mailing list
[email protected]
http://lists.openembedded.org/mailman/listinfo/openembedded-core
