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

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

[Léo Le Bouter via Guix-patches via]

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

[bug#46959] [PATCH] gnu: newlib: Fix CVE-2021-3420.

[Léo Le Bouter via Guix-patches via]

* 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

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

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

[Christopher Baines]

[-- Attachment #1: Type: text/plain, Size: 1917 bytes --]


Léo Le Bouter via Guix-patches via <guix-patches@gnu.org> writes:

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

1: https://patches.guix-patches.cbaines.net/project/guix-patches/patch/20210306050521.11571-1-lle-bout@zaclys.net/

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.

2: https://data.guix-patches.cbaines.net/build-server/5/build?build_server_build_id=dd289414-7653-4b63-8b3c-7a55cdf55820

Any ideas? What packages should build with this change?

Thanks,

Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

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

[Léo Le Bouter via Guix-patches via]

[-- Attachment #1: Type: text/plain, Size: 484 bytes --]

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.

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

Léo

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Christopher Baines]

[-- Attachment #1: Type: text/plain, Size: 491 bytes --]


Léo Le Bouter <lle-bout@zaclys.net> writes:

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]