linux-hardening - Re: [syzbot] [zstd] UBSAN: array-index-out-of-bounds in FSE_decompress_wksp_body

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <202310091025.4939AEBC9@keescook>
Date: Mon, 9 Oct 2023 10:29:25 -0700
From: Kees Cook <keescook@...omium.org>
To: Eric Biggers <ebiggers@...nel.org>
Cc: Nick Terrell <terrelln@...com>,
	syzbot <syzbot+1f2eb3e8cd123ffce499@...kaller.appspotmail.com>,
	clm@...com, dsterba@...e.com, josef@...icpanda.com,
	linux-btrfs@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org, syzkaller-bugs@...glegroups.com,
	linux-hardening@...r.kernel.org
Subject: Re: [syzbot] [zstd] UBSAN: array-index-out-of-bounds in
 FSE_decompress_wksp_body_bmi2

On Sat, Oct 07, 2023 at 02:05:56PM -0700, Eric Biggers wrote:
> Hi Nick,
> 
> On Wed, Aug 30, 2023 at 12:49:53AM -0700, syzbot wrote:
> > UBSAN: array-index-out-of-bounds in lib/zstd/common/fse_decompress.c:345:30
> > index 33 is out of range for type 'FSE_DTable[1]' (aka 'unsigned int[1]')
> 
> Zstandard needs to be converted to use C99 flex-arrays instead of length-1
> arrays.  https://github.com/facebook/zstd/pull/3785 would fix this in upstream
> Zstandard, though it doesn't work well with the fact that upstream Zstandard
> supports C90.  Not sure how you want to handle this.

For the kernel, we just need:

diff --git a/lib/zstd/common/fse_decompress.c b/lib/zstd/common/fse_decompress.c
index a0d06095be83..b11e87fff261 100644
--- a/lib/zstd/common/fse_decompress.c
+++ b/lib/zstd/common/fse_decompress.c
@@ -312,7 +312,7 @@ size_t FSE_decompress_wksp(void* dst, size_t dstCapacity, const void* cSrc, size
 
 typedef struct {
     short ncount[FSE_MAX_SYMBOL_VALUE + 1];
-    FSE_DTable dtable[1]; /* Dynamically sized */
+    FSE_DTable dtable[]; /* Dynamically sized */
 } FSE_DecompressWksp;
 
 
And if upstream wants to stay C89 compat, perhaps:

#if __STDC_VERSION__ >= 199901L
# define __FLEX_ARRAY_DIM	/*C99*/
#else
# define __FLEX_ARRAY_DIM	0
#endif

and then use __FLEX_ARRAY_DIM as needed (and keep the other "-1" changes
in the github commit):

 typedef struct {
     short ncount[FSE_MAX_SYMBOL_VALUE + 1];
-    FSE_DTable dtable[1]; /* Dynamically sized */
+    FSE_DTable dtable[__FLEX_ARRAY_DIM]; /* Dynamically sized */
 } FSE_DecompressWksp;
 

-- 
Kees Cook