Subject: Re: Heap corruption buffer overflow in bsd_probe

From: Natanael Copa <[email-masked]>
Date: Fri, 4 Dec 2020 12:58:54 +0100

On Thu, 3 Dec 2020 13:45:48 -0500
Rich Felker <[email-masked]> 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