Skip to content

Handle empty input in String's FromBytesWithInput implementation#306

Closed
oherrala wants to merge 1 commit intojbaublitz:mainfrom
oherrala:empty-string-from-bytes
Closed

Handle empty input in String's FromBytesWithInput implementation#306
oherrala wants to merge 1 commit intojbaublitz:mainfrom
oherrala:empty-string-from-bytes

Conversation

@oherrala
Copy link
Contributor

@oherrala oherrala commented Oct 11, 2025

This fixes "attempt to subtract with overflow" error when the input buffer is empty. In this case buffer.position() + input - 1 is 0 + 0 - 1 leading to unsigned integer overflow.

In release builds integer overflows are not checked which could lead into out of bounds read of the buffer.

While here validate UTF-8 string before allocating memory for it.

Error from included test before fix is applied:

---- test::test_nl_string_empty stdout ----

thread 'test::test_nl_string_empty' panicked at src/lib.rs:437:46:
attempt to subtract with overflow
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

neli/src/lib.rs

Lines 435 to 439 in 7c6a1f9

let s = String::from_utf8(
buffer.get_ref().as_ref()
[buffer.position() as usize..buffer.position() as usize + input - 1]
.to_vec(),
)?;

This fixes "attempt to subtract with overflow" error when the input
buffer is empty. In this case

  buffer.position() + input - 1

is 0 + 0 - 1 leading to integer overflow.

In release builds integer overflows are not checked which could lead
into out of bounds read of the buffer.

While here validate UTF-8 string before allocating memory for it.
@oherrala oherrala closed this Oct 11, 2025
@oherrala oherrala deleted the empty-string-from-bytes branch October 11, 2025 15:28
@jbaublitz
Copy link
Owner

@oherrala Are you going to resubmit this?

@oherrala
Copy link
Contributor Author

@jbaublitz I was too hasty and misunderstood some of the code. I opened #307 with maybe more understanding of the code on my part. Sorry about the noise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants