Skip to contentSkip to conversation

wpa-supplicant: Fix CVE-2015-8041

PATCH In review Hongxu Jia 5 participants 7 messages Nov 13, 2015 – Feb 29, 2016

lore
Share & tools
lore

Hongxu Jia

author
#0 lore raw

cover wpa-supplicant: Fix CVE-2015-8041

The following changes since commit fc45deac89ef63ca1c44e763c38ced7dfd72cbe1:

build-appliance-image: Update to jethro head revision (2015-11-03 14:03:03 +0000)

are available in the git repository at:

git://git.pokylinux.org/poky-contrib hongxu/wpa-supplicant
http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=hongxu/wpa-supplicant

Hongxu Jia (1):
wpa-supplicant: Fix CVE-2015-8041

 ...load-length-validation-in-NDEF-record-par.patch | 64 ++++++++++++++++++++++
 .../wpa-supplicant/wpa-supplicant_2.4.bb           |  1 +
 2 files changed, 65 insertions(+)
 create mode 100644 meta/recipes-connectivity/wpa-supplicant/wpa-supplicant/0001-NFC-Fix-payload-length-validation-in-NDEF-record-par.patch
Signature · 1 line
1.9.1
author
#1 lore raw

PATCH 1/1 wpa-supplicant: Fix CVE-2015-8041

Backport patch from http://w1.fi/security/2015-5/
and rebase for wpa-supplicant 2.4

---
 ...load-length-validation-in-NDEF-record-par.patch | 64 ++++++++++++++++++++++
 .../wpa-supplicant/wpa-supplicant_2.4.bb           |  1 +
 2 files changed, 65 insertions(+)
 create mode 100644 meta/recipes-connectivity/wpa-supplicant/wpa-supplicant/0001-NFC-Fix-payload-length-validation-in-NDEF-record-par.patch
meta/recipes-connectivity/wpa-supplicant/wpa-supplicant/0001-NFC-Fix-payload-length-validation-in-NDEF-record-par.patch new file +64 -0
new file mode 100644
index 0000000..bc1d1e5
@@ -0,0 +1,64 @@
+From c13401c723a039971bcd91b3856d76c6041b15f2 Mon Sep 17 00:00:00 2001
+From: Jouni Malinen <[email protected]>
+Date: Fri, 13 Nov 2015 05:54:18 -0500
+Subject: [PATCH] NFC: Fix payload length validation in NDEF record parser
+
+It was possible for the 32-bit record->total_length value to end up
+wrapping around due to integer overflow if the longer form of payload
+length field is used and record->payload_length gets a value close to
+2^32. This could result in ndef_parse_record() accepting a too large
+payload length value and the record type filter reading up to about 20
+bytes beyond the end of the buffer and potentially killing the process.
+This could also result in an attempt to allocate close to 2^32 bytes of
+heap memory and if that were to succeed, a buffer read overflow of the
+same length which would most likely result in the process termination.
+In case of record->total_length ending up getting the value 0, there
+would be no buffer read overflow, but record parsing would result in an
+infinite loop in ndef_parse_records().
+
+Any of these error cases could potentially be used for denial of service
+attacks over NFC by using a malformed NDEF record on an NFC Tag or
+sending them during NFC connection handover if the application providing
+the NDEF message to hostapd/wpa_supplicant did no validation of the
+received records. While such validation is likely done in the NFC stack
+that needs to parse the NFC messages before further processing,
+hostapd/wpa_supplicant better be prepared for any data being included
+here.
+
+Fix this by validating record->payload_length value in a way that
+detects integer overflow. (CID 122668)
+
+Signed-off-by: Jouni Malinen <[email protected]>
+
+Upstream-Status: Backport [from http://w1.fi/security/2015-5/]
+Signed-off-by: Hongxu Jia <[email protected]>
+---
+ src/wps/ndef.c | 5 ++++-
+ 1 file changed, 4 insertions(+), 1 deletion(-)
+
+diff --git a/src/wps/ndef.c b/src/wps/ndef.c
+index d45dfc8..f7f729b 100644
+--- a/src/wps/ndef.c
++++ b/src/wps/ndef.c
+@@ -48,6 +48,8 @@ static int ndef_parse_record(const u8 *data, u32 size,
+ 		if (size < 6)
+ 			return -1;
+ 		record->payload_length = ntohl(*(u32 *)pos);
++                if (record->payload_length > size - 6)
++                       return -1;
+ 		pos += sizeof(u32);
+ 	}
+ 
+@@ -68,7 +70,8 @@ static int ndef_parse_record(const u8 *data, u32 size,
+ 	pos += record->payload_length;
+ 
+ 	record->total_length = pos - data;
+-	if (record->total_length > size)
++	if (record->total_length > size ||
++	    record->total_length < record->payload_length)
+ 		return -1;
+ 	return 0;
+ }
+-- 
+1.9.1
+
meta/recipes-connectivity/wpa-supplicant/wpa-supplicant_2.4.bb +1 -0
index a124cf2..6e4d028 100644
@@ -32,6 +32,7 @@ SRC_URI = "http://hostap.epitest.fi/releases/wpa_supplicant-${PV}.tar.gz \
            file://0003-EAP-pwd-peer-Fix-Total-Length-parsing-for-fragment-r.patch \
            file://0004-EAP-pwd-server-Fix-Total-Length-parsing-for-fragment.patch \
            file://0005-EAP-pwd-peer-Fix-asymmetric-fragmentation-behavior.patch \
+           file://0001-NFC-Fix-payload-length-validation-in-NDEF-record-par.patch \
           "
 SRC_URI[md5sum] = "f0037dbe03897dcaf2ad2722e659095d"
 SRC_URI[sha256sum] = "058dc832c096139a059e6df814080f50251a8d313c21b13364c54a1e70109122"
Signature · 1 line
1.9.1
#2 lore raw
Quoted 2 lines · Hongxu Jia

Backport patch from http://w1.fi/security/2015-5/
and rebase for wpa-supplicant 2.4

There's a thread about upgrading master to 2.5 (which should fix this) already.

The patch probably still makes sense for jethro though.

- Jussi

Quoted 1 line
Quoted 110 lines
---
 ...load-length-validation-in-NDEF-record-par.patch | 64
++++++++++++++++++++++
 .../wpa-supplicant/wpa-supplicant_2.4.bb           |  1 +
 2 files changed, 65 insertions(+)
 create mode 100644

meta/recipes-connectivity/wpa-supplicant/wpa-supplicant/0001-NFC-Fix-payload-length-validation-in-NDEF-record-par.patch

diff --git a/meta/recipes-connectivity/wpa-supplicant/wpa-supplicant/0001-NFC-Fix-payload-length-validation-in-NDEF-record-par.patch b/meta/recipes-connectivity/wpa-supplicant/wpa-supplicant/0001-NFC-Fix-payload-length-validation-in-NDEF-record-par.patch new file mode 100644
index 0000000..bc1d1e5
--- /dev/null
+++
b/meta/recipes-connectivity/wpa-supplicant/wpa-supplicant/0001-NFC-Fix-payload-length-validation-in-NDEF-record-par.patch

meta/recipes-connectivity/wpa-supplicant/wpa-supplicant/0001-NFC-Fix-payload-length-validation-in-NDEF-record-par.patch new file +64 -0
@@ -0,0 +1,64 @@
+From c13401c723a039971bcd91b3856d76c6041b15f2 Mon Sep 17 00:00:00 2001
+From: Jouni Malinen <[email protected]>
+Date: Fri, 13 Nov 2015 05:54:18 -0500
+Subject: [PATCH] NFC: Fix payload length validation in NDEF record parser
+
+It was possible for the 32-bit record->total_length value to end up
+wrapping around due to integer overflow if the longer form of payload
+length field is used and record->payload_length gets a value close to
+2^32. This could result in ndef_parse_record() accepting a too large
+payload length value and the record type filter reading up to about 20
+bytes beyond the end of the buffer and potentially killing the process.
+This could also result in an attempt to allocate close to 2^32 bytes of
+heap memory and if that were to succeed, a buffer read overflow of the
+same length which would most likely result in the process termination.
+In case of record->total_length ending up getting the value 0, there
+would be no buffer read overflow, but record parsing would result in an
+infinite loop in ndef_parse_records().
+
+Any of these error cases could potentially be used for denial of service
+attacks over NFC by using a malformed NDEF record on an NFC Tag or
+sending them during NFC connection handover if the application providing
+the NDEF message to hostapd/wpa_supplicant did no validation of the
+received records. While such validation is likely done in the NFC stack
+that needs to parse the NFC messages before further processing,
+hostapd/wpa_supplicant better be prepared for any data being included
+here.
+
+Fix this by validating record->payload_length value in a way that
+detects integer overflow. (CID 122668)
+
+Signed-off-by: Jouni Malinen <[email protected]>
+
+Upstream-Status: Backport [from http://w1.fi/security/2015-5/]
+Signed-off-by: Hongxu Jia <[email protected]>
+---
+ src/wps/ndef.c | 5 ++++-
+ 1 file changed, 4 insertions(+), 1 deletion(-)
+
+diff --git a/src/wps/ndef.c b/src/wps/ndef.c
+index d45dfc8..f7f729b 100644
+--- a/src/wps/ndef.c
++++ b/src/wps/ndef.c
+@@ -48,6 +48,8 @@ static int ndef_parse_record(const u8 *data, u32 size,
+               if (size < 6)
+                       return -1;
+               record->payload_length = ntohl(*(u32 *)pos);
++                if (record->payload_length > size - 6)
++                       return -1;
+               pos += sizeof(u32);
+       }
+
+@@ -68,7 +70,8 @@ static int ndef_parse_record(const u8 *data, u32 size,
+       pos += record->payload_length;
+
+       record->total_length = pos - data;
+-      if (record->total_length > size)
++      if (record->total_length > size ||
++          record->total_length < record->payload_length)
+               return -1;
+       return 0;
+ }
+--
+1.9.1
+
(quoted diff) +0 -0

wpa-supplicant_2.4.bb b/meta/recipes-connectivity/wpa-supplicant/ wpa-supplicant_2.4.bb
index a124cf2..6e4d028 100644

meta/recipes-connectivity/wpa-supplicant/wpa-supplicant_2.4.bb +0 -0
@@ -32,6 +32,7 @@ SRC_URI = "
 http://hostap.epitest.fi/releases/wpa_supplicant-${PV}.tar.gz \
 
 file://0003-EAP-pwd-peer-Fix-Total-Length-parsing-for-fragment-r.patch \
 
 file://0004-EAP-pwd-server-Fix-Total-Length-parsing-for-fragment.patch \
 

file://0005-EAP-pwd-peer-Fix-asymmetric-fragmentation-behavior.patch \
+

 file://0001-NFC-Fix-payload-length-validation-in-NDEF-record-par.patch \
           "

SRC_URI[md5sum] = "f0037dbe03897dcaf2ad2722e659095d" SRC_URI[sha256sum] =
"058dc832c096139a059e6df814080f50251a8d313c21b13364c54a1e70109122" --
1.9.1

--
_______________________________________________
Openembedded-core mailing list
[email protected]
http://lists.openembedded.org/mailman/listinfo/openembedded-core

author
#3 lore raw
Quoted 9 lines · Jussi Kukkonen

On 13 November 2015 at 13:08, Hongxu Jia <[email protected] <mailto:[email protected]>> wrote:

Backport patch from http://w1.fi/security/2015-5/
and rebase for wpa-supplicant 2.4

There's a thread about upgrading master to 2.5 (which should fix this) already. The patch probably still makes sense for jethro though.

Yes, you are right, the 2.5 don't need this, it makes sense for jethro.

//Hongxu

Quoted 140 lines

- Jussi

    Signed-off-by: Hongxu Jia <[email protected]
    <mailto:[email protected]>>
    ---
     ...load-length-validation-in-NDEF-record-par.patch | 64
    ++++++++++++++++++++++
     .../wpa-supplicant/wpa-supplicant_2.4.bb
    <http://wpa-supplicant_2.4.bb>           | 1 +
     2 files changed, 65 insertions(+)
     create mode 100644
    meta/recipes-connectivity/wpa-supplicant/wpa-supplicant/0001-NFC-Fix-payload-length-validation-in-NDEF-record-par.patch

    diff --git
    a/meta/recipes-connectivity/wpa-supplicant/wpa-supplicant/0001-NFC-Fix-payload-length-validation-in-NDEF-record-par.patch
    b/meta/recipes-connectivity/wpa-supplicant/wpa-supplicant/0001-NFC-Fix-payload-length-validation-in-NDEF-record-par.patch
    new file mode 100644
    index 0000000..bc1d1e5
    --- /dev/null
    +++
    b/meta/recipes-connectivity/wpa-supplicant/wpa-supplicant/0001-NFC-Fix-payload-length-validation-in-NDEF-record-par.patch
    @@ -0,0 +1,64 @@
    +From c13401c723a039971bcd91b3856d76c6041b15f2 Mon Sep 17 00:00:00
    2001
    +From: Jouni Malinen <[email protected] <mailto:[email protected]>>
    +Date: Fri, 13 Nov 2015 05:54:18 -0500
    +Subject: [PATCH] NFC: Fix payload length validation in NDEF
    record parser
    +
    +It was possible for the 32-bit record->total_length value to end up
    +wrapping around due to integer overflow if the longer form of payload
    +length field is used and record->payload_length gets a value close to
    +2^32. This could result in ndef_parse_record() accepting a too large
    +payload length value and the record type filter reading up to
    about 20
    +bytes beyond the end of the buffer and potentially killing the
    process.
    +This could also result in an attempt to allocate close to 2^32
    bytes of
    +heap memory and if that were to succeed, a buffer read overflow
    of the
    +same length which would most likely result in the process
    termination.
    +In case of record->total_length ending up getting the value 0, there
    +would be no buffer read overflow, but record parsing would result
    in an
    +infinite loop in ndef_parse_records().
    +
    +Any of these error cases could potentially be used for denial of
    service
    +attacks over NFC by using a malformed NDEF record on an NFC Tag or
    +sending them during NFC connection handover if the application
    providing
    +the NDEF message to hostapd/wpa_supplicant did no validation of the
    +received records. While such validation is likely done in the NFC
    stack
    +that needs to parse the NFC messages before further processing,
    +hostapd/wpa_supplicant better be prepared for any data being included
    +here.
    +
    +Fix this by validating record->payload_length value in a way that
    +detects integer overflow. (CID 122668)
    +
    +Signed-off-by: Jouni Malinen <[email protected] <mailto:[email protected]>>
    +
    +Upstream-Status: Backport [from http://w1.fi/security/2015-5/]
    +Signed-off-by: Hongxu Jia <[email protected]
    <mailto:[email protected]>>
    +---
    + src/wps/ndef.c | 5 ++++-
    + 1 file changed, 4 insertions(+), 1 deletion(-)
    +
    +diff --git a/src/wps/ndef.c b/src/wps/ndef.c
    +index d45dfc8..f7f729b 100644
    +--- a/src/wps/ndef.c
    ++++ b/src/wps/ndef.c
    +@@ -48,6 +48,8 @@ static int ndef_parse_record(const u8 *data,
    u32 size,
    +               if (size < 6)
    +                       return -1;
    +               record->payload_length = ntohl(*(u32 *)pos);
    ++                if (record->payload_length > size - 6)
    ++                       return -1;
    +               pos += sizeof(u32);
    +       }
    +
    +@@ -68,7 +70,8 @@ static int ndef_parse_record(const u8 *data,
    u32 size,
    +       pos += record->payload_length;
    +
    +       record->total_length = pos - data;
    +-      if (record->total_length > size)
    ++      if (record->total_length > size ||
    ++          record->total_length < record->payload_length)
    +               return -1;
    +       return 0;
    + }
    +--
    +1.9.1
    +
    diff --git
    a/meta/recipes-connectivity/wpa-supplicant/wpa-supplicant_2.4.bb
    <http://wpa-supplicant_2.4.bb>
    b/meta/recipes-connectivity/wpa-supplicant/wpa-supplicant_2.4.bb
    <http://wpa-supplicant_2.4.bb>
    index a124cf2..6e4d028 100644
    ---
    a/meta/recipes-connectivity/wpa-supplicant/wpa-supplicant_2.4.bb
    <http://wpa-supplicant_2.4.bb>
    +++
    b/meta/recipes-connectivity/wpa-supplicant/wpa-supplicant_2.4.bb
    <http://wpa-supplicant_2.4.bb>
    @@ -32,6 +32,7 @@ SRC_URI =
    "http://hostap.epitest.fi/releases/wpa_supplicant-${PV}.tar.gz
    <http://hostap.epitest.fi/releases/wpa_supplicant-$%7BPV%7D.tar.gz> \
    file://0003-EAP-pwd-peer-Fix-Total-Length-parsing-for-fragment-r.patch
    \
    file://0004-EAP-pwd-server-Fix-Total-Length-parsing-for-fragment.patch
    \
    file://0005-EAP-pwd-peer-Fix-asymmetric-fragmentation-behavior.patch \
    +
     file://0001-NFC-Fix-payload-length-validation-in-NDEF-record-par.patch
    \
               "
     SRC_URI[md5sum] = "f0037dbe03897dcaf2ad2722e659095d"
     SRC_URI[sha256sum] =
    "058dc832c096139a059e6df814080f50251a8d313c21b13364c54a1e70109122"
    --
    1.9.1

    --
    _______________________________________________
    Openembedded-core mailing list
    [email protected]
    <mailto:[email protected]>
    http://lists.openembedded.org/mailman/listinfo/openembedded-core
#4 lore raw

Hi Robert,

I just noticed this never got merged into jethro. Could you take care of that? The original patch is here:

http://patchwork.openembedded.org/patch/107625/

Joshua, looks like we could use this one in fido as well.

Thanks,
Paul

Quoted 166 lines · On Fri, 13 Nov 2015 20:18:09 Hongxu Jia wrote:
Quoted 9 lines · Jussi Kukkonen

On 13 November 2015 at 13:08, Hongxu Jia <[email protected]

<mailto:[email protected]>> wrote:
Backport patch from http://w1.fi/security/2015-5/
and rebase for wpa-supplicant 2.4

There's a thread about upgrading master to 2.5 (which should fix this) already.
The patch probably still makes sense for jethro though.

Yes, you are right, the 2.5 don't need this, it makes sense for jethro.

//Hongxu

Quoted 151 lines

- Jussi

    Signed-off-by: Hongxu Jia <[email protected]
    <mailto:[email protected]>>
    ---
    
     ...load-length-validation-in-NDEF-record-par.patch | 64
    
    ++++++++++++++++++++++
    
     .../wpa-supplicant/wpa-supplicant_2.4.bb
    
    <http://wpa-supplicant_2.4.bb>           | 1 +
    
     2 files changed, 65 insertions(+)
     create mode 100644
    
    meta/recipes-connectivity/wpa-supplicant/wpa-supplicant/0001-NFC-Fix-p
    ayload-length-validation-in-NDEF-record-par.patch
    
    diff --git
    a/meta/recipes-connectivity/wpa-supplicant/wpa-supplicant/0001-NFC-Fix
    -payload-length-validation-in-NDEF-record-par.patch
    b/meta/recipes-connectivity/wpa-supplicant/wpa-supplicant/0001-NFC-Fi
    x-payload-length-validation-in-NDEF-record-par.patch new file mode
    100644
    index 0000000..bc1d1e5
    --- /dev/null
    +++
    b/meta/recipes-connectivity/wpa-supplicant/wpa-supplicant/0001-NFC-Fix
    -payload-length-validation-in-NDEF-record-par.patch @@ -0,0 +1,64 @@
    +From c13401c723a039971bcd91b3856d76c6041b15f2 Mon Sep 17 00:00:00
    2001
    +From: Jouni Malinen <[email protected] <mailto:[email protected]>>
    +Date: Fri, 13 Nov 2015 05:54:18 -0500
    +Subject: [PATCH] NFC: Fix payload length validation in NDEF
    record parser
    +
    +It was possible for the 32-bit record->total_length value to end up
    +wrapping around due to integer overflow if the longer form of payload
    +length field is used and record->payload_length gets a value close to
    +2^32. This could result in ndef_parse_record() accepting a too large
    +payload length value and the record type filter reading up to
    about 20
    +bytes beyond the end of the buffer and potentially killing the
    process.
    +This could also result in an attempt to allocate close to 2^32
    bytes of
    +heap memory and if that were to succeed, a buffer read overflow
    of the
    +same length which would most likely result in the process
    termination.
    +In case of record->total_length ending up getting the value 0, there
    +would be no buffer read overflow, but record parsing would result
    in an
    +infinite loop in ndef_parse_records().
    +
    +Any of these error cases could potentially be used for denial of
    service
    +attacks over NFC by using a malformed NDEF record on an NFC Tag or
    +sending them during NFC connection handover if the application
    providing
    +the NDEF message to hostapd/wpa_supplicant did no validation of the
    +received records. While such validation is likely done in the NFC
    stack
    +that needs to parse the NFC messages before further processing,
    +hostapd/wpa_supplicant better be prepared for any data being included
    +here.
    +
    +Fix this by validating record->payload_length value in a way that
    +detects integer overflow. (CID 122668)
    +
    +Signed-off-by: Jouni Malinen <[email protected] <mailto:[email protected]>>
    +
    +Upstream-Status: Backport [from http://w1.fi/security/2015-5/]
    +Signed-off-by: Hongxu Jia <[email protected]
    <mailto:[email protected]>>
    +---
    + src/wps/ndef.c | 5 ++++-
    + 1 file changed, 4 insertions(+), 1 deletion(-)
    +
    +diff --git a/src/wps/ndef.c b/src/wps/ndef.c
    +index d45dfc8..f7f729b 100644
    +--- a/src/wps/ndef.c
    ++++ b/src/wps/ndef.c
    +@@ -48,6 +48,8 @@ static int ndef_parse_record(const u8 *data,
    u32 size,
    +               if (size < 6)
    +                       return -1;
    +               record->payload_length = ntohl(*(u32 *)pos);
    ++                if (record->payload_length > size - 6)
    ++                       return -1;
    +               pos += sizeof(u32);
    +       }
    +
    +@@ -68,7 +70,8 @@ static int ndef_parse_record(const u8 *data,
    u32 size,
    +       pos += record->payload_length;
    +
    +       record->total_length = pos - data;
    +-      if (record->total_length > size)
    ++      if (record->total_length > size ||
    ++          record->total_length < record->payload_length)
    +               return -1;
    +       return 0;
    + }
    +--
    +1.9.1
    +
    diff --git
    a/meta/recipes-connectivity/wpa-supplicant/wpa-supplicant_2.4.bb
    <http://wpa-supplicant_2.4.bb>
    b/meta/recipes-connectivity/wpa-supplicant/wpa-supplicant_2.4.bb
    <http://wpa-supplicant_2.4.bb>
    index a124cf2..6e4d028 100644
    ---
    a/meta/recipes-connectivity/wpa-supplicant/wpa-supplicant_2.4.bb
    <http://wpa-supplicant_2.4.bb>
    +++
    b/meta/recipes-connectivity/wpa-supplicant/wpa-supplicant_2.4.bb
    <http://wpa-supplicant_2.4.bb>
    @@ -32,6 +32,7 @@ SRC_URI =
    "http://hostap.epitest.fi/releases/wpa_supplicant-${PV}.tar.gz
    <http://hostap.epitest.fi/releases/wpa_supplicant-$%7BPV%7D.tar.gz> \
    file://0003-EAP-pwd-peer-Fix-Total-Length-parsing-for-fragment-r.patch
    \
    file://0004-EAP-pwd-server-Fix-Total-Length-parsing-for-fragment.patch
    \
    file://0005-EAP-pwd-peer-Fix-asymmetric-fragmentation-behavior.patch \
    +
    
     file://0001-NFC-Fix-payload-length-validation-in-NDEF-record-par.patc
     h
    
    \
    
               "
     
     SRC_URI[md5sum] = "f0037dbe03897dcaf2ad2722e659095d"
     SRC_URI[sha256sum] =
    
    "058dc832c096139a059e6df814080f50251a8d313c21b13364c54a1e70109122"
    --
    1.9.1
    
    --
    _______________________________________________
    Openembedded-core mailing list
    [email protected]
    <mailto:[email protected]>
    http://lists.openembedded.org/mailman/listinfo/openembedded-core
Signature · 2 lines
Paul Eggleton
Intel Open Source Technology Centre
#5 lore raw
Quoted 4 lines · Paul Eggleton

Hi Robert,

I just noticed this never got merged into jethro. Could you take care of that? The original patch is here:

Got it, thanks.

// Robert

Quoted 176 lines

http://patchwork.openembedded.org/patch/107625/

Joshua, looks like we could use this one in fido as well.

Thanks,
Paul

Quoted 166 lines · On Fri, 13 Nov 2015 20:18:09 Hongxu Jia wrote:
Quoted 9 lines · Jussi Kukkonen

On 13 November 2015 at 13:08, Hongxu Jia <[email protected]

<mailto:[email protected]>> wrote:
Backport patch from http://w1.fi/security/2015-5/
and rebase for wpa-supplicant 2.4

There's a thread about upgrading master to 2.5 (which should fix this) already.
The patch probably still makes sense for jethro though.

Yes, you are right, the 2.5 don't need this, it makes sense for jethro.

//Hongxu

Quoted 151 lines

- Jussi

     Signed-off-by: Hongxu Jia <[email protected]
     <mailto:[email protected]>>
     ---

      ...load-length-validation-in-NDEF-record-par.patch | 64

     ++++++++++++++++++++++

      .../wpa-supplicant/wpa-supplicant_2.4.bb

     <http://wpa-supplicant_2.4.bb>           | 1 +

      2 files changed, 65 insertions(+)
      create mode 100644

     meta/recipes-connectivity/wpa-supplicant/wpa-supplicant/0001-NFC-Fix-p
     ayload-length-validation-in-NDEF-record-par.patch

     diff --git
     a/meta/recipes-connectivity/wpa-supplicant/wpa-supplicant/0001-NFC-Fix
     -payload-length-validation-in-NDEF-record-par.patch
     b/meta/recipes-connectivity/wpa-supplicant/wpa-supplicant/0001-NFC-Fi
     x-payload-length-validation-in-NDEF-record-par.patch new file mode
     100644
     index 0000000..bc1d1e5
     --- /dev/null
     +++
     b/meta/recipes-connectivity/wpa-supplicant/wpa-supplicant/0001-NFC-Fix
     -payload-length-validation-in-NDEF-record-par.patch @@ -0,0 +1,64 @@
     +From c13401c723a039971bcd91b3856d76c6041b15f2 Mon Sep 17 00:00:00
     2001
     +From: Jouni Malinen <[email protected] <mailto:[email protected]>>
     +Date: Fri, 13 Nov 2015 05:54:18 -0500
     +Subject: [PATCH] NFC: Fix payload length validation in NDEF
     record parser
     +
     +It was possible for the 32-bit record->total_length value to end up
     +wrapping around due to integer overflow if the longer form of payload
     +length field is used and record->payload_length gets a value close to
     +2^32. This could result in ndef_parse_record() accepting a too large
     +payload length value and the record type filter reading up to
     about 20
     +bytes beyond the end of the buffer and potentially killing the
     process.
     +This could also result in an attempt to allocate close to 2^32
     bytes of
     +heap memory and if that were to succeed, a buffer read overflow
     of the
     +same length which would most likely result in the process
     termination.
     +In case of record->total_length ending up getting the value 0, there
     +would be no buffer read overflow, but record parsing would result
     in an
     +infinite loop in ndef_parse_records().
     +
     +Any of these error cases could potentially be used for denial of
     service
     +attacks over NFC by using a malformed NDEF record on an NFC Tag or
     +sending them during NFC connection handover if the application
     providing
     +the NDEF message to hostapd/wpa_supplicant did no validation of the
     +received records. While such validation is likely done in the NFC
     stack
     +that needs to parse the NFC messages before further processing,
     +hostapd/wpa_supplicant better be prepared for any data being included
     +here.
     +
     +Fix this by validating record->payload_length value in a way that
     +detects integer overflow. (CID 122668)
     +
     +Signed-off-by: Jouni Malinen <[email protected] <mailto:[email protected]>>
     +
     +Upstream-Status: Backport [from http://w1.fi/security/2015-5/]
     +Signed-off-by: Hongxu Jia <[email protected]
     <mailto:[email protected]>>
     +---
     + src/wps/ndef.c | 5 ++++-
     + 1 file changed, 4 insertions(+), 1 deletion(-)
     +
     +diff --git a/src/wps/ndef.c b/src/wps/ndef.c
     +index d45dfc8..f7f729b 100644
     +--- a/src/wps/ndef.c
     ++++ b/src/wps/ndef.c
     +@@ -48,6 +48,8 @@ static int ndef_parse_record(const u8 *data,
     u32 size,
     +               if (size < 6)
     +                       return -1;
     +               record->payload_length = ntohl(*(u32 *)pos);
     ++                if (record->payload_length > size - 6)
     ++                       return -1;
     +               pos += sizeof(u32);
     +       }
     +
     +@@ -68,7 +70,8 @@ static int ndef_parse_record(const u8 *data,
     u32 size,
     +       pos += record->payload_length;
     +
     +       record->total_length = pos - data;
     +-      if (record->total_length > size)
     ++      if (record->total_length > size ||
     ++          record->total_length < record->payload_length)
     +               return -1;
     +       return 0;
     + }
     +--
     +1.9.1
     +
     diff --git
     a/meta/recipes-connectivity/wpa-supplicant/wpa-supplicant_2.4.bb
     <http://wpa-supplicant_2.4.bb>
     b/meta/recipes-connectivity/wpa-supplicant/wpa-supplicant_2.4.bb
     <http://wpa-supplicant_2.4.bb>
     index a124cf2..6e4d028 100644
     ---
     a/meta/recipes-connectivity/wpa-supplicant/wpa-supplicant_2.4.bb
     <http://wpa-supplicant_2.4.bb>
     +++
     b/meta/recipes-connectivity/wpa-supplicant/wpa-supplicant_2.4.bb
     <http://wpa-supplicant_2.4.bb>
     @@ -32,6 +32,7 @@ SRC_URI =
     "http://hostap.epitest.fi/releases/wpa_supplicant-${PV}.tar.gz
     <http://hostap.epitest.fi/releases/wpa_supplicant-$%7BPV%7D.tar.gz> \
     file://0003-EAP-pwd-peer-Fix-Total-Length-parsing-for-fragment-r.patch
     \
     file://0004-EAP-pwd-server-Fix-Total-Length-parsing-for-fragment.patch
     \
     file://0005-EAP-pwd-peer-Fix-asymmetric-fragmentation-behavior.patch \
     +

      file://0001-NFC-Fix-payload-length-validation-in-NDEF-record-par.patc
      h

     \

                "

      SRC_URI[md5sum] = "f0037dbe03897dcaf2ad2722e659095d"
      SRC_URI[sha256sum] =

     "058dc832c096139a059e6df814080f50251a8d313c21b13364c54a1e70109122"
     --
     1.9.1

     --
     _______________________________________________
     Openembedded-core mailing list
     [email protected]
     <mailto:[email protected]>
     http://lists.openembedded.org/mailman/listinfo/openembedded-core
#6 lore raw
In reply to Paul Eggleton
Quoted 9 lines · Paul Eggleton

Hi Robert,

I just noticed this never got merged into jethro. Could you take care of that?
The original patch is here:

http://patchwork.openembedded.org/patch/107625/

Joshua, looks like we could use this one in fido as well.

I was hoping to backport this from Jethro but I haven't seen it land there yet, I've queued it for fido so I don't forget.

Robert, I have a branch for jethro with this patch included — should I submit it?

Regards,

Joshua