GNU bug report logs - #45026
Heap corruption buffer overflow in bsd_probe

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 45026 in the body.
You can then email your comments to 45026 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Rich Felker <dalias <at> libc.org>
To: bug-parted <at> gnu.org
Cc: Natanael Copa <ncopa <at> alpinelinux.org>
Subject: Heap corruption buffer overflow in bsd_probe
Date: Thu, 3 Dec 2020 13:45:48 -0500

Commit a5f69f396713ab8ac1e57458cbb9af552d2c1659 rearranged bsd.c's
bsd_probe function in a way that changed the meaning of the local
variable label, but left alone the call to alpha_bootblock_checksum,
thereby causing the checksum to take place over the wrong range of
bytes and be written 56 bytes past the end of the allocated memory.
The checksum call should probably just be removed as the results don't
seem to be used.

This was discovered via a bug report against the Apline Linux package,
https://gitlab.alpinelinux.org/alpine/aports/-/issues/12161. It
appears we just got really lucky catching this, as only one value well
beyond the end of the allocation is written. It turns out that 64+512
makes up exactly the size of musl/mallocng's next size class over 512,
576, and writing 8 bytes before that clobbers all the consistency
check at the end of the slot and the header of the next slot. However
valgrind also seems to catch the bug when running the test cases.

Message #8 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Natanael Copa <ncopa <at> alpinelinux.org>
To: Rich Felker <dalias <at> libc.org>
Cc: bug-parted <at> gnu.org
Subject: Re: Heap corruption buffer overflow in bsd_probe
Date: Fri, 4 Dec 2020 12:58:54 +0100

On Thu, 3 Dec 2020 13:45:48 -0500
Rich Felker <dalias <at> libc.org> wrote:

> Commit a5f69f396713ab8ac1e57458cbb9af552d2c1659 rearranged bsd.c's
> bsd_probe function in a way that changed the meaning of the local
> variable label, but left alone the call to alpha_bootblock_checksum,
> thereby causing the checksum to take place over the wrong range of
> bytes and be written 56 bytes past the end of the allocated memory.
> The checksum call should probably just be removed as the results don't
> seem to be used.
> 
> This was discovered via a bug report against the Apline Linux package,
> https://gitlab.alpinelinux.org/alpine/aports/-/issues/12161. It
> appears we just got really lucky catching this, as only one value well
> beyond the end of the allocation is written. It turns out that 64+512
> makes up exactly the size of musl/mallocng's next size class over 512,
> 576, and writing 8 bytes before that clobbers all the consistency
> check at the end of the slot and the header of the next slot. However
> valgrind also seems to catch the bug when running the test cases.

I had a look at this and I tried to dig up why the
alpha_bootblock_checksum is called from bsd_probe() at all. I cannot
see any reason why nor could git log give me any clues.


I think this should be a safe fix:

diff --git a/libparted/labels/bsd.c b/libparted/labels/bsd.c
index 8483641..0a2b891 100644
--- a/libparted/labels/bsd.c
+++ b/libparted/labels/bsd.c
@@ -164,8 +164,6 @@ bsd_probe (const PedDevice *dev)
 
        label = &((BSDDiskData*) s0)->label;
 
-       alpha_bootblock_checksum(label);
-
        /* check magic */
         bool found = PED_LE32_TO_CPU (label->d_magic) == BSD_DISKMAGIC;
        free (s0);


-nc

Message #13 received at 45026-close <at> debbugs.gnu.org (full text, mbox):

From: "Brian C. Lane" <bcl <at> redhat.com>
To: Natanael Copa <ncopa <at> alpinelinux.org>
Cc: Rich Felker <dalias <at> libc.org>, 45026-close <at> debbugs.gnu.org
Subject: Re: bug#45026: Heap corruption buffer overflow in bsd_probe
Date: Fri, 4 Dec 2020 17:05:51 -0800

How did you get valgrind to hit that? I'm not seeing it complain about
bsd.c on Fedora.

I've pushed this fix to master.

Brian

-- 
Brian C. Lane (PST8PDT) - weldr.io - lorax - parted - pykickstart

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.