libc: FORTIFY: strncpy: detected read past end of 32-byte buffer

[adamboardman]

I've been compiling lvm2 within the android recovery context, it crashes on many commands (eg vgdisplay) due to the metadata reading past the end of a non-null terminated uuid whilst adding a null terminator to a copy.
It wont actually cause any problems as the next line overwrites the stray random character with a '\0'.

Probably worth fixing upstream too though?

output:
libc: FORTIFY: strncpy: detected read past end of 32-byte buffer

Suggested fix
in:
_check_devs_used_correspond_with_vg
from:
strncpy(vgid, (const char *) vg->id.uuid, sizeof(vgid));
to:
strncpy(vgid, (const char *) vg->id.uuid, ID_LEN);

https://github.com/lvmteam/lvm2/blob/master/lib/metadata/metadata.c#L3448

[zkabelac]

Hi

It's true that using 'sizeof(vgid) - 1' or ID_LEN would be sufficient here,
but I can't imagine how it can cause any crashing of your app ???

Still the buffer is sized as ID_LEN+1 and '\0' is written after the call strncpy().

Can you rather share you 'coredump' trace or at least -vvvv log
before your execution crashes - as it looks like reason might be somewhere else.

[adamboardman]

The write side is fine as you say, its the read from vg->id.uuid that whilst is normally fine, it reads an extra byte which is later overwritten. In the specific case of running within the android recovery context (which enforces the use of FORTIFY) does a test on both the read and write side of strncpy and so causes the crash.

[zkabelac]

Ahh ok - so it's purely crashing your enforced fortified build - not a normal build - since the access is normally embedded into a volume_group structure so it should not cause any troubles.
I'll push minor change later on.

[zkabelac]

Changed upstream with this commit:
https://listman.redhat.com/archives/lvm-devel/2021-August/msg00016.html