NetBSD Problem Report #57973

From gson@gson.org  Thu Feb 29 17:00:28 2024
Return-Path: <gson@gson.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id E81B71A923B
	for <gnats-bugs@gnats.NetBSD.org>; Thu, 29 Feb 2024 17:00:27 +0000 (UTC)
Message-Id: <20240229170018.3D4E62546A9@guava.gson.org>
Date: Thu, 29 Feb 2024 19:00:18 +0200 (EET)
From: gson@gson.org (Andreas Gustafsson)
Reply-To: gson@gson.org (Andreas Gustafsson)
To: gnats-bugs@NetBSD.org
Subject: Read buffer overflow in audioplay(1)
X-Send-Pr-Version: 3.95

>Number:         57973
>Category:       bin
>Synopsis:       Read buffer overflow in audioplay(1)
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    mrg
>State:          needs-pullups
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Feb 29 17:05:00 +0000 2024
>Closed-Date:    
>Last-Modified:  Mon Mar 11 19:20:01 +0000 2024
>Originator:     Andreas Gustafsson
>Release:        NetBSD-current as of yesterday
>Organization:
>Environment:
System: NetBSD
Architecture: x86_64
Machine: amd64
>Description:

I am in need of some BSD licensed code for reading .wav files and
figured I could reuse that in NetBSD's src/usr.bin/audio/common/wav.c,
used by audioplay(1), but after reading some of it, I don't think I
want to run it on untrusted input.

In audio_wav_parse_hdr(), there's this loop:

	do {
		memcpy(&part, where, sizeof part);
		owhere = where;
		where += getle32(part.len) + 8;
	} while (where < end && strncmp(part.name, strfmt, sizeof strfmt));

where the memcpy reads from "where" without checking there is enough
data first; that's a read buffer overflow.

This is followed by

	/* too short ? */
	if (where + sizeof fmt > end)
		return (AUDIO_ESHORTHDR);

Here, the pointer addition has undefined behavior when the "if"
condition would otherwise be true (assuming the array "where" points
into actually ends at "end").

>How-To-Repeat:

Code inspection.

>Fix:

>Release-Note:

>Audit-Trail:

Responsible-Changed-From-To: bin-bug-people->mrg
Responsible-Changed-By: mrg@NetBSD.org
Responsible-Changed-When: Sat, 02 Mar 2024 00:27:05 +0000
Responsible-Changed-Why:
mine.


From: "matthew green" <mrg@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/57973 CVS commit: src/usr.bin/audio/common
Date: Fri, 8 Mar 2024 06:57:59 +0000

 Module Name:	src
 Committed By:	mrg
 Date:		Fri Mar  8 06:57:59 UTC 2024

 Modified Files:
 	src/usr.bin/audio/common: libaudio.h wav.c

 Log Message:
 audio_wav_parse_hdr(): avoid buffer overreads and clean up

 reimplement most of this function using a new method of buffer
 management to ensure that we never read beyond the provided size.

 properly handle RIFF chunk lengths, instead of assuming various
 offsets from most files are right.

 update list of consumed documentation and fill the list of WAVE
 formats from RFC 2361 (most remain not supported.)

 should fix PR#57973.

 tested against a large number of .wav files i have handy and with
 a testsuite generator that should be incoming soon.


 To generate a diff of this commit:
 cvs rdiff -u -r1.21 -r1.22 src/usr.bin/audio/common/libaudio.h
 cvs rdiff -u -r1.18 -r1.19 src/usr.bin/audio/common/wav.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->closed
State-Changed-By: gson@NetBSD.org
State-Changed-When: Mon, 11 Mar 2024 11:36:16 +0000
State-Changed-Why:
I believe the overruns are now fixed.  Thanks!


From: Taylor R Campbell <riastradh@NetBSD.org>
To: gson@NetBSD.org, mrg@NetBSD.org
Cc: gnats-bugs@NetBSD.org, netbsd-bugs@NetBSD.org
Subject: Re: bin/57973: Read buffer overflow in audioplay(1)
Date: Mon, 11 Mar 2024 11:45:27 +0000

 Should these audioplay(1) changes be pulled up to 10, 9, 8?

State-Changed-From-To: closed->needs-pullups
State-Changed-By: gson@NetBSD.org
State-Changed-When: Mon, 11 Mar 2024 12:03:36 +0000
State-Changed-Why:
Yes, some pullups would probably be a good idea.


From: matthew green <mrg@eterna23.net>
To: gnats-bugs@netbsd.org
Cc: netbsd-bugs@netbsd.org, gnats-admin@netbsd.org, gson@NetBSD.org,
    gson@gson.org (Andreas Gustafsson)
Subject: re: bin/57973 (Read buffer overflow in audioplay(1))
Date: Tue, 12 Mar 2024 06:19:04 +1100

 > Yes, some pullups would probably be a good idea.

 ah, need to add wav.c 1.20 as well -- rillig pointed out a new bug
 (but only in a warning.)

 thanks for checking -- i'll send pullups.


 .mrg.

>Unformatted:

Home	PR Database Search