[PATCH 0/1] WIP: gnu: newlib: Fix CVE-2021-3420.

OpenSubmitted by Léo Le Bouter.
Details
2 participants
  • Léo Le Bouter
  • Christopher Baines
Owner
unassigned
Severity
normal
L
L
Léo Le Bouter wrote on 6 Mar 2021 06:04
(address . guix-patches@gnu.org)(name . Léo Le Bouter)(address . lle-bout@zaclys.net)
20210306050410.11022-1-lle-bout@zaclys.net
newlib-CVE-2021-3420.patch needs backporting to the versions of newlib it is
being applied to, so if you are interested or a user of those packages please
finish the work, otherwise well CVE-2021-3420 will probably remain unfixed.

The versions of newlib are too old and too specific for it to be
maintainable security-wise, especially considering upstream does not seem to
maintain older versions at all. I don't think GNU Guix should take that role,
but of course the people who depend on these packages can ensure they are good
enough for themselves, otherwise contribute changes.

Léo Le Bouter (1):
gnu: newlib: Fix CVE-2021-3420.

gnu/local.mk | 1 +
gnu/packages/embedded.scm | 6 +-
.../patches/newlib-CVE-2021-3420.patch | 105 ++++++++++++++++++
3 files changed, 110 insertions(+), 2 deletions(-)
create mode 100644 gnu/packages/patches/newlib-CVE-2021-3420.patch

--
2.30.1
L
L
Léo Le Bouter wrote on 6 Mar 2021 06:05
[PATCH] gnu: newlib: Fix CVE-2021-3420.
(address . 46959@debbugs.gnu.org)(name . Léo Le Bouter)(address . lle-bout@zaclys.net)
20210306050521.11571-1-lle-bout@zaclys.net
* gnu/packages/patches/newlib-CVE-2021-3420.patch: New patch.
* gnu/local.mk (dist_patch_DATA): Register it.
* gnu/packages/embedded.scm (newlib-arm-none-eabi,
newlib-arm-none-eabi-7-2018-q2-update): Apply it.
---
gnu/local.mk | 1 +
gnu/packages/embedded.scm | 6 +-
.../patches/newlib-CVE-2021-3420.patch | 105 ++++++++++++++++++
3 files changed, 110 insertions(+), 2 deletions(-)
create mode 100644 gnu/packages/patches/newlib-CVE-2021-3420.patch

Toggle diff (149 lines)
diff --git a/gnu/local.mk b/gnu/local.mk
index fb3b395852..d0260b5921 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -1397,6 +1397,7 @@ dist_patch_DATA =						\
   %D%/packages/patches/netsurf-system-utf8proc.patch		\
   %D%/packages/patches/netsurf-y2038-tests.patch		\
   %D%/packages/patches/netsurf-longer-test-timeout.patch	\
+  %D%/packages/patches/newlib-CVE-2021-3420.patch		\
   %D%/packages/patches/nfs4-acl-tools-0.3.7-fixpaths.patch	\
   %D%/packages/patches/ngircd-handle-zombies.patch		\
   %D%/packages/patches/network-manager-plugin-path.patch	\
diff --git a/gnu/packages/embedded.scm b/gnu/packages/embedded.scm
index 51ee244f3c..72dbdf7385 100644
--- a/gnu/packages/embedded.scm
+++ b/gnu/packages/embedded.scm
@@ -173,7 +173,8 @@
                                   version ".tar.gz"))
               (sha256
                (base32
-                "01i7qllwicf05vsvh39qj7qp5fdifpvvky0x95hjq39mbqiksnsl"))))
+                "01i7qllwicf05vsvh39qj7qp5fdifpvvky0x95hjq39mbqiksnsl"))
+              (patches (search-patches "newlib-CVE-2021-3420.patch"))))
     (build-system gnu-build-system)
     (arguments
      `(#:out-of-source? #t
@@ -339,7 +340,8 @@ usable on embedded products.")
          (file-name (git-file-name "newlib" commit))
          (sha256
           (base32
-           "1dq23fqrk75g1a4v7569fvnnw5q440zawbxi3w0g05n8jlqsmvcy"))))
+           "1dq23fqrk75g1a4v7569fvnnw5q440zawbxi3w0g05n8jlqsmvcy"))
+         (patches (search-patches "newlib-CVE-2021-3420.patch"))))
       (arguments
        (substitute-keyword-arguments (package-arguments newlib-arm-none-eabi)
          ;; The configure flags are identical to the flags used by the "GCC ARM
diff --git a/gnu/packages/patches/newlib-CVE-2021-3420.patch b/gnu/packages/patches/newlib-CVE-2021-3420.patch
new file mode 100644
index 0000000000..f7834664b5
--- /dev/null
+++ b/gnu/packages/patches/newlib-CVE-2021-3420.patch
@@ -0,0 +1,105 @@
+From aa106b29a6a8a1b0df9e334704292cbc32f2d44e Mon Sep 17 00:00:00 2001
+From: Corinna Vinschen <vinschen@redhat.com>
+Date: Tue, 17 Nov 2020 10:50:57 +0100
+Subject: [PATCH] malloc/nano-malloc: correctly check for out-of-bounds
+ allocation reqs
+
+The overflow check in mEMALIGn erroneously checks for INT_MAX,
+albeit the input parameter is size_t.  Fix this to check for
+__SIZE_MAX__ instead.  Also, it misses to check the req against
+adding the alignment before calling mALLOc.
+
+While at it, add out-of-bounds checks to pvALLOc, nano_memalign,
+nano_valloc, and Cygwin's (unused) dlpvalloc.
+
+Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
+---
+ newlib/libc/stdlib/mallocr.c      |  7 ++++++-
+ newlib/libc/stdlib/nano-mallocr.c | 22 +++++++++++++++++++++-
+ winsup/cygwin/malloc.cc           |  4 ++++
+ 3 files changed, 31 insertions(+), 2 deletions(-)
+
+diff --git a/newlib/libc/stdlib/mallocr.c b/newlib/libc/stdlib/mallocr.c
+index 9ad720ada..13d014cc8 100644
+--- a/newlib/libc/stdlib/mallocr.c
++++ b/newlib/libc/stdlib/mallocr.c
+@@ -3055,7 +3055,7 @@ Void_t* mEMALIGn(RARG alignment, bytes) RDECL size_t alignment; size_t bytes;
+   nb = request2size(bytes);
+ 
+   /* Check for overflow. */
+-  if (nb > INT_MAX || nb < bytes)
++  if (nb > __SIZE_MAX__ - (alignment + MINSIZE) || nb < bytes)
+   {
+     RERRNO = ENOMEM;
+     return 0;
+@@ -3172,6 +3172,11 @@ Void_t* pvALLOc(RARG bytes) RDECL size_t bytes;
+ #endif
+ {
+   size_t pagesize = malloc_getpagesize;
++  if (bytes > __SIZE_MAX__ - pagesize)
++  {
++    RERRNO = ENOMEM;
++    return 0;
++  }
+   return mEMALIGn (RCALL pagesize, (bytes + pagesize - 1) & ~(pagesize - 1));
+ }
+ 
+diff --git a/newlib/libc/stdlib/nano-mallocr.c b/newlib/libc/stdlib/nano-mallocr.c
+index 6dbfba84b..1e0703948 100644
+--- a/newlib/libc/stdlib/nano-mallocr.c
++++ b/newlib/libc/stdlib/nano-mallocr.c
+@@ -580,8 +580,22 @@ void * nano_memalign(RARG size_t align, size_t s)
+     if ((align & (align-1)) != 0) return NULL;
+ 
+     align = MAX(align, MALLOC_ALIGN);
++
++    /* Make sure ma_size does not overflow */
++    if (s > __SIZE_MAX__ - CHUNK_ALIGN)
++    {
++	RERRNO = ENOMEM;
++	return NULL;
++    }
+     ma_size = ALIGN_SIZE(MAX(s, MALLOC_MINSIZE), CHUNK_ALIGN);
+-    size_with_padding = ma_size + align - MALLOC_ALIGN;
++
++    /* Make sure size_with_padding does not overflow */
++    if (ma_size > __SIZE_MAX__ - (align - MALLOC_ALIGN))
++    {
++	RERRNO = ENOMEM;
++	return NULL;
++    }
++    size_with_padding = ma_size + (align - MALLOC_ALIGN);
+ 
+     allocated = nano_malloc(RCALL size_with_padding);
+     if (allocated == NULL) return NULL;
+@@ -644,6 +658,12 @@ void * nano_valloc(RARG size_t s)
+ #ifdef DEFINE_PVALLOC
+ void * nano_pvalloc(RARG size_t s)
+ {
++    /* Make sure size given to nano_valloc does not overflow */
++    if (s > __SIZE_MAX__ - MALLOC_PAGE_ALIGN)
++    {
++	RERRNO = ENOMEM;
++	return NULL;
++    }
+     return nano_valloc(RCALL ALIGN_SIZE(s, MALLOC_PAGE_ALIGN));
+ }
+ #endif /* DEFINE_PVALLOC */
+diff --git a/winsup/cygwin/malloc.cc b/winsup/cygwin/malloc.cc
+index 23c354074..8a1fc257e 100644
+--- a/winsup/cygwin/malloc.cc
++++ b/winsup/cygwin/malloc.cc
+@@ -5298,6 +5298,10 @@ void* dlpvalloc(size_t bytes) {
+   size_t pagesz;
+   ensure_initialization();
+   pagesz = mparams.page_size;
++  if (bytes > MAX_REQUEST) {
++    MALLOC_FAILURE_ACTION;
++    return NULL;
++  }
+   return dlmemalign(pagesz, (bytes + pagesz - SIZE_T_ONE) & ~(pagesz - SIZE_T_ONE));
+ }
+ 
+-- 
+2.27.0
+
-- 
2.30.1
C
C
Christopher Baines wrote on 7 Mar 2021 14:57
Re: [bug#46959] [PATCH 0/1] WIP: gnu: newlib: Fix CVE-2021-3420.
(name . Léo Le Bouter)(address . lle-bout@zaclys.net)(address . 46959@debbugs.gnu.org)
871rcrnk26.fsf@cbaines.net
Léo Le Bouter via Guix-patches via <guix-patches@gnu.org> writes:

Toggle quote (19 lines)
> newlib-CVE-2021-3420.patch needs backporting to the versions of newlib it is
> being applied to, so if you are interested or a user of those packages please
> finish the work, otherwise well CVE-2021-3420 will probably remain unfixed.
>
> The versions of newlib are too old and too specific for it to be
> maintainable security-wise, especially considering upstream does not seem to
> maintain older versions at all. I don't think GNU Guix should take that role,
> but of course the people who depend on these packages can ensure they are good
> enough for themselves, otherwise contribute changes.
>
> Léo Le Bouter (1):
> gnu: newlib: Fix CVE-2021-3420.
>
> gnu/local.mk | 1 +
> gnu/packages/embedded.scm | 6 +-
> .../patches/newlib-CVE-2021-3420.patch | 105 ++++++++++++++++++
> 3 files changed, 110 insertions(+), 2 deletions(-)
> create mode 100644 gnu/packages/patches/newlib-CVE-2021-3420.patch

Hey,

Looking at [1] and following through the "View comparison" links, it
seems that there's some problems applying the patch added here, I can't
see a case where it's applied successfully.


Unfortunately this data is still a bit hidden, but if you click on
"Compare package derivations", get all the results, then find
newlib@3.0.0-0.3ccfb40 and look at the build for x86_64-linux, you
should get to this page [2] and from the "Required failed builds", I'm
guessing the source part of the package build has failed.


Any ideas? What packages should build with this change?

Thanks,

Chris
-----BEGIN PGP SIGNATURE-----

iQKlBAEBCgCPFiEEPonu50WOcg2XVOCyXiijOwuE9XcFAmBE27FfFIAAAAAALgAo
aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNF
ODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcRHG1haWxAY2Jh
aW5lcy5uZXQACgkQXiijOwuE9XcYbg/9FtshF5jHipG1Hj77sHNp1IwinQU4rJxD
RTpdebExGr6ojVsBx0SlpBriVEmS7icA+7O4rhmGxxJapmn+tYSaJqMx2io/tzIn
tv9wjq2t0qrMCNRMA3CQWoW54yaKBIJOn/UezNCxEnfb63KPhQP6Ulg2iC6z8g3U
3S2bQM2LeC/XD0pojz3TyLre8hyabv7jzDpUGj1JEbpxK7+n2ypKiPO2TxeRu4tm
0ln9hiq3sr5+lKHfVhLz0rtDIFr3LxiuyI7t4OwKR8T49TMiDI9R9wyUqVpOzMRC
EdcIxQKiviVCw+dbQke0fxRNvlqEexV3fLA9QV94K9z2OFowYwgAtXF1jYy7mqWU
/taDlaBf0uWUBx//x8jMOOsQm0l7M/r1MVVtfaLDrNN5LiSNzly0n6fhDNlr7wQM
1e5HTdSGacAWBlmpuWKgpMsJLbiRN4QbPz98ukVqCMDoCxoHzfKLpQT9kVLjS7Pz
u3qcWlt1ICWtPd1sm7tuxuGqIGjzgj6oBF6FvUjx8S9E+s1TVymNVWkwSR9EQaFF
lcQ8D06Ux9NOSGT37865xNgBRlpQzH0ogyqlSNNupk+hw7GLjjtsAN4QE7Sqma/C
Cvxo8b/hx6F4132uZOEt1jA5ILR+Bh7wpV4o9baBIkHD+NL5kciSubs+6KL0fzy7
s7ur4mP1GIA=
=RYxy
-----END PGP SIGNATURE-----

L
L
Léo Le Bouter wrote on 9 Mar 2021 06:17
(name . Christopher Baines)(address . mail@cbaines.net)(address . 46959@debbugs.gnu.org)
3c481b2024c6d2b56afe403814c25472f15e1afe.camel@zaclys.net
Hello!

On Sun, 2021-03-07 at 13:57 +0000, Christopher Baines wrote:
Toggle quote (2 lines)
> Any ideas? What packages should build with this change?

If you are saying that this patch I sent here breaks the builds
(because the newlib-CVE-2021-3420.patch does not apply), yes this is
intended, I do not have motivation to finish work here, newlib-CVE-
2021-3420.patch needs to be backported to the versions we package.

I do not know if these packages are actually used by anyone either.

Léo
-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEFIvLi9gL+xax3g6RRaix6GvNEKYFAmBHBQAACgkQRaix6GvN
EKZ0ehAAtY0gUsSgVuJBu42EZImz42Lq93sGw4BVctZH0XxAxljn1eV5iXMHZJ6M
zTUlWvUwTgszHYJuZyA4mM9KXk3zBf6Z6HbhBak/w+9hoYPQM94OBq2ItdnxkTOF
HLKSy9wpUTZBXShuzsXaYhm2z+zSv4eGJeoNbDtAiR1/02ttvG3rDvUiz9o1rAs7
7T/x4CQxw6sPhXje7cJZ+txWHCgs7G4yglEKIeppAXzCQWsqDSv1WplzdxEpwldV
AeT3AK0aRGykVC04pWp3z7PzoU4svInGdWdz/3dB0ZBULWN/LnkM1AuXKFSbmRd3
ZGG5wbwfhOyfbchE+PucxujuCBqb6ELtVMRwx9lV7N/C753gjGRyBdUEiXnDaDU6
tCV7nWakRO+jinTc+a9RMBA1/rWx9p7WjHwR+pdjwEwbACj67jddLKEYZsd1ibSd
Q5noLjBl6mw0nQRjjOIyT6qgPS9MHk7ZGvMa29t1LIXbWeh1gN1TsJAyM8FOkzBE
G7u0EH82pEDaK5jnAUCf0N1ap05L5O54rXniXnVd1wo9WQ6+xYt+Ct6eXUZxjj3K
rnahY33JAYWc83ZMC4cKspZ4ClLFb5cVF3b9Th9DEXUgiMcdR35oSpFI1nsxRyY3
lpYsGa9pn5YXKpQX8lP2k+XNTheGnfSVr7dDLHwucjrfkO5AAIk=
=8rrB
-----END PGP SIGNATURE-----


C
C
Christopher Baines wrote on 9 Mar 2021 08:58
(name . Léo Le Bouter)(address . lle-bout@zaclys.net)(address . 46959@debbugs.gnu.org)
875z20bvxh.fsf@cbaines.net
Léo Le Bouter <lle-bout@zaclys.net> writes:

Toggle quote (10 lines)
> Hello!
>
> On Sun, 2021-03-07 at 13:57 +0000, Christopher Baines wrote:
>> Any ideas? What packages should build with this change?
>
> If you are saying that this patch I sent here breaks the builds
> (because the newlib-CVE-2021-3420.patch does not apply), yes this is
> intended, I do not have motivation to finish work here, newlib-CVE-
> 2021-3420.patch needs to be backported to the versions we package.

Ok, good to know.
-----BEGIN PGP SIGNATURE-----

iQKlBAEBCgCPFiEEPonu50WOcg2XVOCyXiijOwuE9XcFAmBHKppfFIAAAAAALgAo
aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNF
ODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcRHG1haWxAY2Jh
aW5lcy5uZXQACgkQXiijOwuE9XftdA/+KSrptSuJjbAE8lDGkN+jbrecEeMO1ch5
SYfJUtq8atFpSho6+LCQfhh14n9BJQrfVL4sgBIh0w9x8EyXeClk1n1gaCpqPGke
BLEjSdada/Mo8gs11/VaFDxNvAcB1P7oYPeOfIg/5XUB94etGwM7AIEttcjEG7yH
Ee7RXDOo+DIkGKtGLniED8zdmYtZnMEXa96O6UIRIXkB8+I+OXFjkdYru8BVV0SK
Gd1GzsQL860k51713Lv4gN+K6RlMhi1cdeT0Rd+jsGgCjLleUcpjO8UJ4h9tDYpS
9i6aeuXpsbtkFa/BxfEIvsSUg/E9xBF1JDt+cCf1U4pfh/jJdw+FRWArUwk2bbuQ
L97vobk1x0WpJzV4paQ5tPddE9T4c6sBZOGior6LFSGJLo7+Ih2e7BICU7BH3/1c
B9h8zXGhuPs2VV2gvdSV07JTZqUTDGM0TmuGsAQ42K643tiBZzBaVXscy+4aqdAi
MMRNq4F3LHZ7y3T9Ux92lI+2qO2+XyvOUg6Mj7+ZBt9SWdKEOxD5bGq5Nu7pFzHO
+bjNgjWqWFGW2lgUIpyHTDBcYOwfPyqy81dXpML6IG5tw+LeLHnhUsP+GyGxHJRD
whXv7S6C6P2GUP3iwpwUZFI/lrXJILJUIxvdjvdRewmu/Rq+ROnhA3BucehMPF0z
EioJ15a9wMk=
=IOS2
-----END PGP SIGNATURE-----

L
L
Léo Le Bouter wrote on 19 Mar 2021 11:28
(address . control@debbugs.gnu.org)
d69095f35860c0eb4b6a212f6409d259e7e7da06.camel@zaclys.net
tags 46959 + security
quit
-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEFIvLi9gL+xax3g6RRaix6GvNEKYFAmBUfOkACgkQRaix6GvN
EKZkLxAAucd1hLImCk6oh63YNP6L3eATZ4jNdwZzO6ygvLag3NfjbAXOcsm6paUR
B4DBgKyuBrxlvswkz0GFVZlB/0TWueqVY9d9cgAIB0XpIYj7T5lZWrKRjF8AgJ5V
1kSYqXRNJiiZZqxbqKxxjsLYmp8KZMbgSUGRjM5Dxt7e6A6xYrL24KybGcZjKl3v
3glY7haXPJTVj1Grn29CyiBEe2x0jti9DtoijyKZZWrkZeFxBAZLkKhk0CCIuSN7
FUN+wQmxuBPgAUbSeJKjDVDhrIS2wz+PkZb+KCcby+tJuBqqSXhDDRnFbtwk+Kyn
ssCbNTIQlW2IEt1jqwHAmbDateiwZI2dKOXUCuWKKCrrti4iTLPeLQd6WWBkZkRi
fVMFd7fkJK96m6L2saz3U/RkbUgwpNAlijP1qnCjLEyLe85RmRM4hycoctnwad9Q
94g2b1TxPnUqfq/cxQBhU1patrW4TkgGYfXGdvyd1QalO1pWYwZLkSTgTcPY5gLC
fH7firxlqYgAaSppAn7adDBcTmMmP+CWmPNNVSLhRBXq/MJK2e+ZKJNeEDYF7YT7
8rHf73gxvgvtaSEHbE9/LLunrWxRMG5K/B95outX8BEfu8iWIOHj6WJaMEnZtpot
gmvrDChbzTbpwo6Z7/LTsxxpPtkQTERCvCHtESNe2Q6zBOedWZs=
=1HXV
-----END PGP SIGNATURE-----


?