mgeisler / textwrap

An efficient and powerful Rust library for word wrapping text.
MIT License
446 stars 44 forks source link

Arithmetic overflow found on wrap_columns() #517

Open HeeillWang opened 12 months ago

HeeillWang commented 12 months ago

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);
mgeisler commented 12 months ago

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.

HeeillWang commented 12 months ago

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

mgeisler commented 10 months ago

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.