Fix intra-object buffer overread for labeled msdosfs volumes

[jrtc27]

Summary

Volume labels, like directory entries, are padded with spaces and so
have no NUL terminator. Whilst the MIN for the dsize argument to strlcpy
ensures that the copy does not overflow the destination, strlcpy is
defined to return the number of characters in the source string,
regardless of the provided dsize, and so keeps reading until it finds a
NUL, which likely exists somewhere within the following fields, but On
CHERI with the subobject bounds enabled in the compiler this buffer
overread will be detected and trap with a bounds violation.

Found by: CHERI

[trasz]

Might be worth making the same change to usr/sbin/fstyp/msdosfs.c; it's largely copy/pasted from here.

[imp]

I like it. Is there a coccinelle script we can run on the whole tree?

[jrtc27]

In D32579#735324, @imp wrote:

I like it. Is there a coccinelle script we can run on the whole tree?

Presumably the transform isn't hard to write for someone who knows coccinelle, but the issue is knowing whether a string is from a trusted source or not (in the sense that we can trust it to have a NUL byte; strlcpy is safe if you know there's a NUL in the object but just cannot guarantee a bound on its length). For that you'd presumably need taint tracking via annotations on every data structure.

[jrtc27]

Apply identical patch to fstyp

[jrtc27]

In D32579#735297, @trasz wrote:

Might be worth making the same change to usr/sbin/fstyp/msdosfs.c; it's largely copy/pasted from here.

Indeed, git show | patch usr.sbin/fstyp/msdosfs.c just worked :D

[imp]

In D32579#735327, @jrtc27 wrote:

Presumably the transform isn't hard to write for someone who knows coccinelle, but the issue is knowing whether a string is from a trusted source or not (in the sense that we can trust it to have a NUL byte; strlcpy is safe if you know there's a NUL in the object but just cannot guarantee a bound on its length). For that you'd presumably need taint tracking via annotations on every data structure.

I'll take a stab at writing it.

Without a tag, coccinelle is a bit limited to looking for access via a structure...

[imp]

The following script

// xxx
@@
FAT_BSBPB *bsb;
expression E1, E2;
@@
- strlcpy(E1,bsb->BS_VolLab,E2)
+ THIS_IS_BOGUS(E1,bsb->BS_VolLab,E2)

@@
FAT32_BSBPB *bsb;
expression E1, E2;
@@
- strlcpy(E1,bsb->BS_VolLab,E2)
+ THIS_IS_BOGUS(E1,bsb->BS_VolLab,E2)

Found no further instances of this in the tree.

[imp]

This comment was removed by imp.