Hi Pádraig, Thank you for reviewing this and for adding the regression test and NEWS entry.
Best regards, Guanqiang Han At 2026-07-11 02:15:52, "Pádraig Brady" <[email protected]> wrote: >On 10/07/2026 15:18, h wrote: >> Hello coreutils maintainers, >> >> I sent this report four days ago, but it does not appear in the >> bug-coreutils archive, so I am resending it after subscribing to the mailing >> list. >> >> The details are provided below: >> >> I noticed a possible robustness issue in GNU pr. >> >> In src/pr.c, the -e tab width is parsed as an int, and very large positive >> values are accepted. Later, when expanding tab characters, char_to_clump() >> computes the tab expansion width and updates the current input position >> using int arithmetic: >> >> width = TAB_WIDTH(chars_per_input_tab, input_position); >> ... >> input_position += width; >> >> With a very large tab width and enough tab characters in the input, this >> position update can exceed INT_MAX. In a normal build this may not produce a >> visible failure, but with UBSan enabled it can report signed integer >> overflow. >> >> For example, with a UBSan build: >> >> printf '\t\t\t\t\t\t\t\t\t\n' > /tmp/tabs.txt >> ./src/pr -t -e268435456 /tmp/tabs.txt > /dev/null >> >> This also implies very large memory allocation and output processing, since >> clump_buff is allocated based on chars_per_input_tab. >> >> I am not sure whether this should be considered a bug or just an extreme >> input case. >> >> >> I prepared a small patch for this issue. >> >> The patch guards the input_position += width update in char_to_clump() with >> ckd_add(), >> so that an integer overflow is reported instead of relying on undefined >> signed overflow behavior. >> >> Please let me know if this approach looks reasonable. >> >> Note: The patch author address is my other email address. >> >> Best regards, >> >> Guanqiang Han > >Interestingly your repro triggers heap corruption >in the i18n patched version. I.e., on Fedora 44 I see: > > $ valgrind pr -t -e268435456 tabs.txt > /dev/null > > Invalid write of size 8 > at 0x4864064: memset (vg_replace_strmem.c:1399) > by 0x4003CA9: char_to_clump_multi (pr.c:3010) >Note ckd_add doesn't need a temp variable, >and can operate directly on a single variable. > >This should also have a test and NEWS, >which I've done in the proposed patch attached, >which I'll push soon. > >Marking this as done. > >thanks, >Padraig
