Buffer overflow in vfs_parse_ls_lga

[mc-butler]

Important

This issue was migrated from Trac:

Origin	https://midnight-commander.org/ticket/4642
Reporter	zaytsev (@zyv)

Found in Alpine/musl on s390x, confirmed on aarch64 using valgrind - introduced in [65a7278]:

==156518== Invalid read of size 1
==156518==    at 0x413BE0: vfs_parse_ls_lga (parse_ls_vga.c:863)
==156518==    by 0x4076C3: process_ls_line (mc_parse_ls_l.c:350)
==156518==    by 0x4076C3: process_input (mc_parse_ls_l.c:376)
==156518==    by 0x40736B: main (mc_parse_ls_l.c:404)
==156518==  Address 0x536be6f is 1 bytes before a block of size 2 alloc'd
==156518==    at 0x48854F0: malloc (vg_replace_malloc.c:446)
==156518==    by 0x4CF4FCB: g_malloc (gmem.c:100)
==156518==    by 0x4D0E99B: g_strdup (gstrfuncs.c:323)
==156518==    by 0x413887: g_strdup_inline (gstrfuncs.h:321)
==156518==    by 0x413887: vfs_parse_ls_lga (parse_ls_vga.c:848)
==156518==    by 0x4076C3: process_ls_line (mc_parse_ls_l.c:350)
==156518==    by 0x4076C3: process_input (mc_parse_ls_l.c:376)
==156518==    by 0x40736B: main (mc_parse_ls_l.c:404)

https://gitlab.alpinelinux.org/alpine/aports/-/merge_requests/79071

[mc-butler]

Changed by zaytsev (@zyv) on Jan 30, 2025 at 13:15 UTC (comment 1)

• Status changed from new to accepted
• Owner set to zaytsev

[mc-butler]

Changed by zaytsev (@zyv) on Jan 30, 2025 at 13:19 UTC (comment 2)

• Description edited

[mc-butler]

Changed by zaytsev (@zyv) on Jan 30, 2025 at 14:46 UTC (comment 3)

• Branch state changed from no branch to on review

Branch: 4642_fix_overflow
Changeset: [1807962]

Caused by int -> size_t conversion and --p2 > 0 expression. I don't remove consecutive \n\n, \r\r and \n\r, only \n, \r and \r\n. My understanding is that this is the desired behavior.

Please ignore the formatting, I will rebase after #4592.

[mc-butler]

Changed by andrew_b (@aborodin) on Jan 30, 2025 at 17:13 UTC (comment 4)

Some suggestions

• find a more specific name for str_chomp(), since it remove a single trailing EOL not all;
• rename tests/lib/strutil/strutil.c to tests/lib/strutil/str_chomp.c;
• fix copyright year and "Written by" in tests/lib/strutil/strutil.c.

[mc-butler]

Changed by zaytsev (@zyv) on Feb 3, 2025 at 9:57 UTC (comment 5)

Rebased on current master: [ac9a81d]

find a more specific name for str_chomp(), since it remove a single trailing EOL not all;

str_rstrip_eol like Python's rstrip ?

rename tests/lib/strutil/strutil.c to tests/lib/strutil/str_chomp.c;

done

fix copyright year and "Written by" in tests/lib/strutil/strutil.c.

done

[mc-butler]

Changed by andrew_b (@aborodin) on Feb 3, 2025 at 17:13 UTC (comment 6)

• Votes set to andrew_b
• Branch state changed from on review to approved

[mc-butler]

Changed by zaytsev (@zyv) on Feb 3, 2025 at 17:16 UTC (comment 7)

• Branch state changed from approved to merged
• Status changed from accepted to testing
• Votes changed from andrew_b to committed-master
• Resolution set to fixed

Merged as [5cae1dd] .

[mc-butler]

Changed by zaytsev (@zyv) on Feb 3, 2025 at 17:17 UTC (comment 8)

• Status changed from testing to closed