textwrap icon indicating copy to clipboard operation
textwrap copied to clipboard

Arithmetic overflow found on wrap_columns()

Open HeeillWang opened this issue 1 year ago • 3 comments

I executed fuzzing with textwrap-0.16.0. Proper panic message should be added by assert! or need to use checked operations.

Thread '<unnamed>' panicked at 'attempt to multiply with overflow', textwrap-0.16.0/src/lib.rs:1173
pub fn wrap_columns<'a, Opt>(
    text: &str,
    columns: usize,
    total_width_or_options: Opt,
    left_gap: &str,
    middle_gap: &str,
    right_gap: &str,
) -> Vec<String>
where
    Opt: Into<Options<'a>>,
{
    assert!(columns > 0);

    let mut options: Options = total_width_or_options.into();

    let inner_width = options
        .width
        .saturating_sub(core::display_width(left_gap))
        .saturating_sub(core::display_width(right_gap))
        .saturating_sub(core::display_width(middle_gap) * (columns - 1));   // overflow!

reproduce with :

let mut fuzz_arg0: &str = "J";
let mut fuzz_arg1: &str = "\u{8}\n\0?@";
let mut fuzz_arg2: &str = "";
let mut fuzz_arg3: usize = 17788374102109585368;
let mut fuzz_arg4: &str = "";
let mut fuzz_arg5: usize = 0;
wrap_columns(fuzz_arg0, fuzz_arg3, fuzz_arg5, fuzz_arg2, fuzz_arg1, fuzz_arg4);

HeeillWang avatar Sep 19 '23 02:09 HeeillWang

Hi @HeeillWang, thanks for reporting this!

Proper panic message should be added by assert! or need to use checked operations.

Could you either add an asset that forbids using a width of zero of add the checked arithmetic? I think it's okay to document that the width should be non-zero and just reject such input since it doesn't make sense.

mgeisler avatar Sep 19 '23 05:09 mgeisler

It seems like the overflow occurs because size of column is too big, not because width is zero.

HeeillWang avatar Sep 19 '23 06:09 HeeillWang

It seems like the overflow occurs because size of column is too big, not because width is zero.

Hey @HeeillWang, I haven't debugged this further — if you make a PR for this, then I would be very happy to include it.

mgeisler avatar Oct 29 '23 20:10 mgeisler