diff mbox series

[dunfell] expat: Backport fix for CVE-2024-28757

Message ID 20240313200343.15817-1-asharma@mvista.com
State Changes Requested
Delegated to: Steve Sakoman
Headers show
Series [dunfell] expat: Backport fix for CVE-2024-28757 | expand

Commit Message

Ashish Sharma March 13, 2024, 8:03 p.m. UTC 
Upstream ref:
https://github.com/libexpat/libexpat/pull/842
https://github.com/libexpat/libexpat/issues/839

Upstream-Status: Backport [https://github.com/libexpat/libexpat/commit/072eca0b72373da103ce15f8f62d1d7b52695454]
Signed-off-by: Ashish Sharma <asharma@mvista.com>
---
 .../expat/expat/CVE-2024-28757.patch          | 57 +++++++++++++++++++
 meta/recipes-core/expat/expat_2.2.9.bb        |  1 +
 2 files changed, 58 insertions(+)
 create mode 100644 meta/recipes-core/expat/expat/CVE-2024-28757.patch

Comments

Mittal, Anuj March 15, 2024, 3:25 a.m. UTC | #1 
On Thu, 2024-03-14 at 01:33 +0530, Ashish Sharma via
lists.openembedded.org wrote:
> Upstream ref:
> https://github.com/libexpat/libexpat/pull/842
> https://github.com/libexpat/libexpat/issues/839
> 
> Upstream-Status: Backport
> [https://github.com/libexpat/libexpat/commit/072eca0b72373da103ce15f8
> f62d1d7b52695454]

This link here and in the patch points to incorrect change. Should it
be:

https://github.com/libexpat/libexpat/pull/842/commits/1d50b80cf31de87750103656f6eb693746854aa8

?

> Signed-off-by: Ashish Sharma <asharma@mvista.com>
> ---
>  .../expat/expat/CVE-2024-28757.patch          | 57
> +++++++++++++++++++
>  meta/recipes-core/expat/expat_2.2.9.bb        |  1 +
>  2 files changed, 58 insertions(+)
>  create mode 100644 meta/recipes-core/expat/expat/CVE-2024-
> 28757.patch
> 
> diff --git a/meta/recipes-core/expat/expat/CVE-2024-28757.patch
> b/meta/recipes-core/expat/expat/CVE-2024-28757.patch
> new file mode 100644
> index 0000000000..c4bdb4621a
> --- /dev/null
> +++ b/meta/recipes-core/expat/expat/CVE-2024-28757.patch
> @@ -0,0 +1,57 @@
> +From 1d50b80cf31de87750103656f6eb693746854aa8 Mon Sep 17 00:00:00
> 2001
> +From: Sebastian Pipping <sebastian@pipping.org>
> +Date: Mon, 4 Mar 2024 23:49:06 +0100
> +Subject: [PATCH] lib/xmlparse.c: Detect billion laughs attack with
> isolated
> + external parser
> +
> +When parsing DTD content with code like ..
> +
> +  XML_Parser parser = XML_ParserCreate(NULL);
> +  XML_Parser ext_parser = XML_ExternalEntityParserCreate(parser,
> NULL, NULL);
> +  enum XML_Status status = XML_Parse(ext_parser, doc,
> (int)strlen(doc), XML_TRUE);
> +
> +.. there are 0 bytes accounted as direct input and all input from
> `doc` accounted
> +as indirect input.  Now function accountingGetCurrentAmplification
> cannot calculate
> +the current amplification ratio as "(direct + indirect) / direct",
> and it did refuse
> +to divide by 0 as one would expect, but it returned 1.0 for this
> case to indicate
> +no amplification over direct input.  As a result, billion laughs
> attacks from
> +DTD-only input were not detected with this isolated way of using an
> external parser.
> +
> +The new approach is to assume direct input of length not 0 but 22 --
> derived from
> +ghost input "<!ENTITY a SYSTEM 'b'>", the shortest possible way to
> include an external
> +DTD --, and do the usual "(direct + indirect) / direct" math with
> "direct := 22".
> +
> +GitHub issue #839 has more details on this issue and its origin in
> ClusterFuzz
> +finding 66812.
> +---
> +CVE: CVE-2024-28757
> +Upstream-Status: Backport
> [https://github.com/libexpat/libexpat/commit/072eca0b72373da103ce15f8
> f62d1d7b52695454]
> +Signed-off-by: Ashish Sharma <asharma@mvista.com>
> +---
> + expat/lib/xmlparse.c | 6 +++++-
> + 1 file changed, 5 insertions(+), 1 deletion(-)
> +
> +diff --git a/expat/lib/xmlparse.c b/expat/lib/xmlparse.c
> +index b884d82b5..d44baa68d 100644
> +--- a/expat/lib/xmlparse.c
> ++++ b/expat/lib/xmlparse.c
> +@@ -7787,6 +7787,8 @@ copyString(const XML_Char *s, const
> XML_Memory_Handling_Suite *memsuite) {
> + 
> + static float
> + accountingGetCurrentAmplification(XML_Parser rootParser) {
> ++  //                                         
> 1.........1.........12 => 22
> ++  const size_t lenOfShortestInclude = sizeof("<!ENTITY a SYSTEM
> 'b'>") - 1;
> +   const XmlBigCount countBytesOutput
> +       = rootParser->m_accounting.countBytesDirect
> +         + rootParser->m_accounting.countBytesIndirect;
> +@@ -7794,7 +7796,9 @@ accountingGetCurrentAmplification(XML_Parser
> rootParser) {
> +       = rootParser->m_accounting.countBytesDirect
> +             ? (countBytesOutput
> +                / (float)(rootParser-
> >m_accounting.countBytesDirect))
> +-            : 1.0f;
> ++            : ((lenOfShortestInclude
> ++                + rootParser->m_accounting.countBytesIndirect)
> ++               / (float)lenOfShortestInclude);
> +   assert(! rootParser->m_parentParser);
> +   return amplificationFactor;
> + }
> diff --git a/meta/recipes-core/expat/expat_2.2.9.bb b/meta/recipes-
> core/expat/expat_2.2.9.bb
> index 8a5006e59a..ea50533ed9 100644
> --- a/meta/recipes-core/expat/expat_2.2.9.bb
> +++ b/meta/recipes-core/expat/expat_2.2.9.bb
> @@ -22,6 +22,7 @@ SRC_URI =
> "git://github.com/libexpat/libexpat.git;protocol=https;branch=master
> \
>             file://libtool-tag.patch \
>             file://CVE-2022-40674.patch \
>             file://CVE-2022-43680.patch \
> +           file://CVE-2024-28757.patch \
>           "
>  
>  SRCREV = "a7bc26b69768f7fb24f0c7976fae24b157b85b13"
> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#197079):
> https://lists.openembedded.org/g/openembedded-core/message/197079
> Mute This Topic: https://lists.openembedded.org/mt/104913576/3616702
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe:
> https://lists.openembedded.org/g/openembedded-core/unsub [
> anuj.mittal@intel.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Steve Sakoman March 15, 2024, 3:36 p.m. UTC | #2 
Fails at build time:

ERROR: expat-2.2.9-r0 do_patch: Applying patch 'CVE-2024-28757.patch'
on target directory
'/home/steve/builds/poky-contrib-dunfell/build/tmp/work/core2-64-poky-linux/expat/2.2.9-r0/git/expat'
Command Error: 'quilt --quiltrc
/home/steve/builds/poky-contrib-dunfell/build/tmp/work/core2-64-poky-linux/expat/2.2.9-r0/recipe-sysroot-native/etc/quiltrc
push' exited with 0  Output:
Applying patch CVE-2024-28757.patch
can't find file to patch at input line 38
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|From 1d50b80cf31de87750103656f6eb693746854aa8 Mon Sep 17 00:00:00 2001
|From: Sebastian Pipping <sebastian@pipping.org>
|Date: Mon, 4 Mar 2024 23:49:06 +0100
|Subject: [PATCH] lib/xmlparse.c: Detect billion laughs attack with isolated
| external parser
|
|When parsing DTD content with code like ..
|
|  XML_Parser parser = XML_ParserCreate(NULL);
|  XML_Parser ext_parser = XML_ExternalEntityParserCreate(parser, NULL, NULL);
|  enum XML_Status status = XML_Parse(ext_parser, doc,
(int)strlen(doc), XML_TRUE);
|
|.. there are 0 bytes accounted as direct input and all input from
`doc` accounted
|as indirect input.  Now function accountingGetCurrentAmplification
cannot calculate
|the current amplification ratio as "(direct + indirect) / direct",
and it did refuse
|to divide by 0 as one would expect, but it returned 1.0 for this case
to indicate
|no amplification over direct input.  As a result, billion laughs attacks from
|DTD-only input were not detected with this isolated way of using an
external parser.
|
|The new approach is to assume direct input of length not 0 but 22 --
derived from
|ghost input "<!ENTITY a SYSTEM 'b'>", the shortest possible way to
include an external
|DTD --, and do the usual "(direct + indirect) / direct" math with
"direct := 22".
|
|GitHub issue #839 has more details on this issue and its origin in ClusterFuzz
|finding 66812.
|---
|CVE: CVE-2024-28757
|Upstream-Status: Backport
[https://github.com/libexpat/libexpat/commit/072eca0b72373da103ce15f8f62d1d7b52695454]
|Signed-off-by: Ashish Sharma <asharma@mvista.com>
|---
| expat/lib/xmlparse.c | 6 +++++-
| 1 file changed, 5 insertions(+), 1 deletion(-)
|
|diff --git a/expat/lib/xmlparse.c b/expat/lib/xmlparse.c
|index b884d82b5..d44baa68d 100644
|--- a/expat/lib/xmlparse.c
|+++ b/expat/lib/xmlparse.c
--------------------------
No file to patch.  Skipping patch.
2 out of 2 hunks ignored
Patch CVE-2024-28757.patch does not apply (enforce with -f)

On Wed, Mar 13, 2024 at 10:03 AM Ashish Sharma via
lists.openembedded.org <asharma=mvista.com@lists.openembedded.org>
wrote:
>
> Upstream ref:
> https://github.com/libexpat/libexpat/pull/842
> https://github.com/libexpat/libexpat/issues/839
>
> Upstream-Status: Backport [https://github.com/libexpat/libexpat/commit/072eca0b72373da103ce15f8f62d1d7b52695454]
> Signed-off-by: Ashish Sharma <asharma@mvista.com>
> ---
>  .../expat/expat/CVE-2024-28757.patch          | 57 +++++++++++++++++++
>  meta/recipes-core/expat/expat_2.2.9.bb        |  1 +
>  2 files changed, 58 insertions(+)
>  create mode 100644 meta/recipes-core/expat/expat/CVE-2024-28757.patch
>
> diff --git a/meta/recipes-core/expat/expat/CVE-2024-28757.patch b/meta/recipes-core/expat/expat/CVE-2024-28757.patch
> new file mode 100644
> index 0000000000..c4bdb4621a
> --- /dev/null
> +++ b/meta/recipes-core/expat/expat/CVE-2024-28757.patch
> @@ -0,0 +1,57 @@
> +From 1d50b80cf31de87750103656f6eb693746854aa8 Mon Sep 17 00:00:00 2001
> +From: Sebastian Pipping <sebastian@pipping.org>
> +Date: Mon, 4 Mar 2024 23:49:06 +0100
> +Subject: [PATCH] lib/xmlparse.c: Detect billion laughs attack with isolated
> + external parser
> +
> +When parsing DTD content with code like ..
> +
> +  XML_Parser parser = XML_ParserCreate(NULL);
> +  XML_Parser ext_parser = XML_ExternalEntityParserCreate(parser, NULL, NULL);
> +  enum XML_Status status = XML_Parse(ext_parser, doc, (int)strlen(doc), XML_TRUE);
> +
> +.. there are 0 bytes accounted as direct input and all input from `doc` accounted
> +as indirect input.  Now function accountingGetCurrentAmplification cannot calculate
> +the current amplification ratio as "(direct + indirect) / direct", and it did refuse
> +to divide by 0 as one would expect, but it returned 1.0 for this case to indicate
> +no amplification over direct input.  As a result, billion laughs attacks from
> +DTD-only input were not detected with this isolated way of using an external parser.
> +
> +The new approach is to assume direct input of length not 0 but 22 -- derived from
> +ghost input "<!ENTITY a SYSTEM 'b'>", the shortest possible way to include an external
> +DTD --, and do the usual "(direct + indirect) / direct" math with "direct := 22".
> +
> +GitHub issue #839 has more details on this issue and its origin in ClusterFuzz
> +finding 66812.
> +---
> +CVE: CVE-2024-28757
> +Upstream-Status: Backport [https://github.com/libexpat/libexpat/commit/072eca0b72373da103ce15f8f62d1d7b52695454]
> +Signed-off-by: Ashish Sharma <asharma@mvista.com>
> +---
> + expat/lib/xmlparse.c | 6 +++++-
> + 1 file changed, 5 insertions(+), 1 deletion(-)
> +
> +diff --git a/expat/lib/xmlparse.c b/expat/lib/xmlparse.c
> +index b884d82b5..d44baa68d 100644
> +--- a/expat/lib/xmlparse.c
> ++++ b/expat/lib/xmlparse.c
> +@@ -7787,6 +7787,8 @@ copyString(const XML_Char *s, const XML_Memory_Handling_Suite *memsuite) {
> +
> + static float
> + accountingGetCurrentAmplification(XML_Parser rootParser) {
> ++  //                                          1.........1.........12 => 22
> ++  const size_t lenOfShortestInclude = sizeof("<!ENTITY a SYSTEM 'b'>") - 1;
> +   const XmlBigCount countBytesOutput
> +       = rootParser->m_accounting.countBytesDirect
> +         + rootParser->m_accounting.countBytesIndirect;
> +@@ -7794,7 +7796,9 @@ accountingGetCurrentAmplification(XML_Parser rootParser) {
> +       = rootParser->m_accounting.countBytesDirect
> +             ? (countBytesOutput
> +                / (float)(rootParser->m_accounting.countBytesDirect))
> +-            : 1.0f;
> ++            : ((lenOfShortestInclude
> ++                + rootParser->m_accounting.countBytesIndirect)
> ++               / (float)lenOfShortestInclude);
> +   assert(! rootParser->m_parentParser);
> +   return amplificationFactor;
> + }
> diff --git a/meta/recipes-core/expat/expat_2.2.9.bb b/meta/recipes-core/expat/expat_2.2.9.bb
> index 8a5006e59a..ea50533ed9 100644
> --- a/meta/recipes-core/expat/expat_2.2.9.bb
> +++ b/meta/recipes-core/expat/expat_2.2.9.bb
> @@ -22,6 +22,7 @@ SRC_URI = "git://github.com/libexpat/libexpat.git;protocol=https;branch=master \
>             file://libtool-tag.patch \
>             file://CVE-2022-40674.patch \
>             file://CVE-2022-43680.patch \
> +           file://CVE-2024-28757.patch \
>           "
>
>  SRCREV = "a7bc26b69768f7fb24f0c7976fae24b157b85b13"
> --
> 2.24.4
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#197079): https://lists.openembedded.org/g/openembedded-core/message/197079
> Mute This Topic: https://lists.openembedded.org/mt/104913576/3620601
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [steve@sakoman.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
diff mbox series

Patch

diff --git a/meta/recipes-core/expat/expat/CVE-2024-28757.patch b/meta/recipes-core/expat/expat/CVE-2024-28757.patch
new file mode 100644
index 0000000000..c4bdb4621a
--- /dev/null
+++ b/meta/recipes-core/expat/expat/CVE-2024-28757.patch
@@ -0,0 +1,57 @@ 
+From 1d50b80cf31de87750103656f6eb693746854aa8 Mon Sep 17 00:00:00 2001
+From: Sebastian Pipping <sebastian@pipping.org>
+Date: Mon, 4 Mar 2024 23:49:06 +0100
+Subject: [PATCH] lib/xmlparse.c: Detect billion laughs attack with isolated
+ external parser
+
+When parsing DTD content with code like ..
+
+  XML_Parser parser = XML_ParserCreate(NULL);
+  XML_Parser ext_parser = XML_ExternalEntityParserCreate(parser, NULL, NULL);
+  enum XML_Status status = XML_Parse(ext_parser, doc, (int)strlen(doc), XML_TRUE);
+
+.. there are 0 bytes accounted as direct input and all input from `doc` accounted
+as indirect input.  Now function accountingGetCurrentAmplification cannot calculate
+the current amplification ratio as "(direct + indirect) / direct", and it did refuse
+to divide by 0 as one would expect, but it returned 1.0 for this case to indicate
+no amplification over direct input.  As a result, billion laughs attacks from
+DTD-only input were not detected with this isolated way of using an external parser.
+
+The new approach is to assume direct input of length not 0 but 22 -- derived from
+ghost input "<!ENTITY a SYSTEM 'b'>", the shortest possible way to include an external
+DTD --, and do the usual "(direct + indirect) / direct" math with "direct := 22".
+
+GitHub issue #839 has more details on this issue and its origin in ClusterFuzz
+finding 66812.
+---
+CVE: CVE-2024-28757
+Upstream-Status: Backport [https://github.com/libexpat/libexpat/commit/072eca0b72373da103ce15f8f62d1d7b52695454]
+Signed-off-by: Ashish Sharma <asharma@mvista.com>
+---
+ expat/lib/xmlparse.c | 6 +++++-
+ 1 file changed, 5 insertions(+), 1 deletion(-)
+
+diff --git a/expat/lib/xmlparse.c b/expat/lib/xmlparse.c
+index b884d82b5..d44baa68d 100644
+--- a/expat/lib/xmlparse.c
++++ b/expat/lib/xmlparse.c
+@@ -7787,6 +7787,8 @@ copyString(const XML_Char *s, const XML_Memory_Handling_Suite *memsuite) {
+ 
+ static float
+ accountingGetCurrentAmplification(XML_Parser rootParser) {
++  //                                          1.........1.........12 => 22
++  const size_t lenOfShortestInclude = sizeof("<!ENTITY a SYSTEM 'b'>") - 1;
+   const XmlBigCount countBytesOutput
+       = rootParser->m_accounting.countBytesDirect
+         + rootParser->m_accounting.countBytesIndirect;
+@@ -7794,7 +7796,9 @@ accountingGetCurrentAmplification(XML_Parser rootParser) {
+       = rootParser->m_accounting.countBytesDirect
+             ? (countBytesOutput
+                / (float)(rootParser->m_accounting.countBytesDirect))
+-            : 1.0f;
++            : ((lenOfShortestInclude
++                + rootParser->m_accounting.countBytesIndirect)
++               / (float)lenOfShortestInclude);
+   assert(! rootParser->m_parentParser);
+   return amplificationFactor;
+ }
diff --git a/meta/recipes-core/expat/expat_2.2.9.bb b/meta/recipes-core/expat/expat_2.2.9.bb
index 8a5006e59a..ea50533ed9 100644
--- a/meta/recipes-core/expat/expat_2.2.9.bb
+++ b/meta/recipes-core/expat/expat_2.2.9.bb
@@ -22,6 +22,7 @@  SRC_URI = "git://github.com/libexpat/libexpat.git;protocol=https;branch=master \
            file://libtool-tag.patch \
            file://CVE-2022-40674.patch \
            file://CVE-2022-43680.patch \
+           file://CVE-2024-28757.patch \
          "
 
 SRCREV = "a7bc26b69768f7fb24f0c7976fae24b157b85b13"