Skip to content

head: fix overflow errors #7721

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sylvestre merged 4 commits into from
Apr 13, 2025
Merged

head: fix overflow errors #7721

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d466b67
head: fix overflow errors
Qelxiros Apr 11, 2025
1ffaf2d
head: update 32-bit tests
Qelxiros Apr 11, 2025
cc11126
head: return to parse::<usize> and switch to parse_size_u64_max
Qelxiros Apr 12, 2025
37eee0e
head: saturate to max int size on 32 bit platforms
Qelxiros Apr 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 6 additions & 18 deletions src/uu/head/src/head.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,16 +191,10 @@ fn arg_iterate<'a>(
if let Some(s) = second.to_str() {
match parse::parse_obsolete(s) {
Some(Ok(iter)) => Ok(Box::new(vec![first].into_iter().chain(iter).chain(args))),
Some(Err(e)) => match e {
parse::ParseError::Syntax => Err(HeadError::ParseError(format!(
"bad argument format: {}",
s.quote()
))),
parse::ParseError::Overflow => Err(HeadError::ParseError(format!(
"invalid argument: {} Value too large for defined datatype",
s.quote()
))),
},
Some(Err(parse::ParseError)) => Err(HeadError::ParseError(format!(
"bad argument format: {}",
s.quote()
))),
None => Ok(Box::new(vec![first, second].into_iter().chain(args))),
}
} else {
Expand Down Expand Up @@ -288,13 +282,7 @@ fn read_n_lines(input: &mut impl io::BufRead, n: u64, separator: u8) -> io::Resu
}

fn catch_too_large_numbers_in_backwards_bytes_or_lines(n: u64) -> Option<usize> {
Copy link
Collaborator

@jfinkels jfinkels Apr 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's just remove this helper function entirely and use the usize::try_from(n).ok() directly where needed.

match usize::try_from(n) {
Ok(value) => Some(value),
Err(e) => {
show!(HeadError::NumTooLarge(e));
None
}
}
usize::try_from(n).ok()
}

fn read_but_last_n_bytes(mut input: impl Read, n: u64) -> io::Result<u64> {
Expand Down Expand Up @@ -668,7 +656,7 @@ mod tests {
//test that bad obsoletes are an error
assert!(arg_outputs("head -123FooBar").is_err());
//test overflow
assert!(arg_outputs("head -100000000000000000000000000000000000000000").is_err());
assert!(arg_outputs("head -100000000000000000000000000000000000000000").is_ok());
//test that empty args remain unchanged
assert_eq!(arg_outputs("head"), Ok("head".to_owned()));
}
Expand Down
148 changes: 78 additions & 70 deletions src/uu/head/src/parse.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,33 +4,37 @@
// file that was distributed with this source code.

use std::ffi::OsString;
use uucore::parser::parse_size::{ParseSizeError, parse_size_u64};
use uucore::parser::parse_size::{ParseSizeError, parse_size_u64_max};

#[derive(PartialEq, Eq, Debug)]
pub enum ParseError {
Syntax,
Overflow,
}
pub struct ParseError;

/// Parses obsolete syntax
/// head -NUM\[kmzv\] // spell-checker:disable-line
pub fn parse_obsolete(src: &str) -> Option<Result<Vec<OsString>, ParseError>> {
let mut chars = src.char_indices();
if let Some((_, '-')) = chars.next() {
let mut num_end = 0usize;
if let Some((mut num_start, '-')) = chars.next() {
num_start += 1;
let mut num_end = src.len();
let mut has_num = false;
let mut plus_possible = false;
let mut last_char = 0 as char;
for (n, c) in &mut chars {
if c.is_ascii_digit() {
has_num = true;
num_end = n;
plus_possible = false;
} else if c == '+' && plus_possible {
plus_possible = false;
num_start += 1;
continue;
} else {
num_end = n;
last_char = c;
break;
}
}
if has_num {
process_num_block(&src[1..=num_end], last_char, &mut chars)
process_num_block(&src[num_start..num_end], last_char, &mut chars)
} else {
None
}
Expand All @@ -45,64 +49,62 @@ fn process_num_block(
last_char: char,
chars: &mut std::str::CharIndices,
) -> Option<Result<Vec<OsString>, ParseError>> {
match src.parse::<usize>() {
Ok(num) => {
let mut quiet = false;
let mut verbose = false;
let mut zero_terminated = false;
let mut multiplier = None;
let mut c = last_char;
loop {
// note that here, we only match lower case 'k', 'c', and 'm'
match c {
// we want to preserve order
// this also saves us 1 heap allocation
'q' => {
quiet = true;
verbose = false;
}
'v' => {
verbose = true;
quiet = false;
}
'z' => zero_terminated = true,
'c' => multiplier = Some(1),
'b' => multiplier = Some(512),
'k' => multiplier = Some(1024),
'm' => multiplier = Some(1024 * 1024),
'\0' => {}
_ => return Some(Err(ParseError::Syntax)),
}
if let Some((_, next)) = chars.next() {
c = next;
} else {
break;
}
}
let mut options = Vec::new();
if quiet {
options.push(OsString::from("-q"));
}
if verbose {
options.push(OsString::from("-v"));
let num = match src.parse::<usize>() {
Ok(n) => n,
Err(e) if *e.kind() == std::num::IntErrorKind::PosOverflow => usize::MAX,
_ => return Some(Err(ParseError)),
};
let mut quiet = false;
let mut verbose = false;
let mut zero_terminated = false;
let mut multiplier = None;
let mut c = last_char;
loop {
// note that here, we only match lower case 'k', 'c', and 'm'
match c {
// we want to preserve order
// this also saves us 1 heap allocation
'q' => {
quiet = true;
verbose = false;
}
if zero_terminated {
options.push(OsString::from("-z"));
'v' => {
verbose = true;
quiet = false;
}
if let Some(n) = multiplier {
options.push(OsString::from("-c"));
let Some(num) = num.checked_mul(n) else {
return Some(Err(ParseError::Overflow));
};
options.push(OsString::from(format!("{num}")));
} else {
options.push(OsString::from("-n"));
options.push(OsString::from(format!("{num}")));
}
Some(Ok(options))
'z' => zero_terminated = true,
'c' => multiplier = Some(1),
'b' => multiplier = Some(512),
'k' => multiplier = Some(1024),
'm' => multiplier = Some(1024 * 1024),
'\0' => {}
_ => return Some(Err(ParseError)),
}
if let Some((_, next)) = chars.next() {
c = next;
} else {
break;
}
Err(_) => Some(Err(ParseError::Overflow)),
}
let mut options = Vec::new();
if quiet {
options.push(OsString::from("-q"));
}
if verbose {
options.push(OsString::from("-v"));
}
if zero_terminated {
options.push(OsString::from("-z"));
}
if let Some(n) = multiplier {
options.push(OsString::from("-c"));
let num = num.saturating_mul(n);
options.push(OsString::from(format!("{num}")));
} else {
options.push(OsString::from("-n"));
options.push(OsString::from(format!("{num}")));
}
Some(Ok(options))
}

/// Parses an -c or -n argument,
Expand All @@ -128,7 +130,7 @@ pub fn parse_num(src: &str) -> Result<(u64, bool), ParseSizeError> {
if trimmed_string.is_empty() {
Ok((0, all_but_last))
} else {
parse_size_u64(trimmed_string).map(|n| (n, all_but_last))
parse_size_u64_max(trimmed_string).map(|n| (n, all_but_last))
}
}

Expand Down Expand Up @@ -177,8 +179,8 @@ mod tests {

#[test]
fn test_parse_errors_obsolete() {
assert_eq!(obsolete("-5n"), Some(Err(ParseError::Syntax)));
assert_eq!(obsolete("-5c5"), Some(Err(ParseError::Syntax)));
assert_eq!(obsolete("-5n"), Some(Err(ParseError)));
assert_eq!(obsolete("-5c5"), Some(Err(ParseError)));
}

#[test]
Expand All @@ -192,18 +194,24 @@ mod tests {
fn test_parse_obsolete_overflow_x64() {
assert_eq!(
obsolete("-1000000000000000m"),
Some(Err(ParseError::Overflow))
obsolete_result(&["-c", "18446744073709551615"])
);
assert_eq!(
obsolete("-10000000000000000000000"),
Some(Err(ParseError::Overflow))
obsolete_result(&["-n", "18446744073709551615"])
);
}

#[test]
#[cfg(target_pointer_width = "32")]
fn test_parse_obsolete_overflow_x32() {
assert_eq!(obsolete("-42949672960"), Some(Err(ParseError::Overflow)));
assert_eq!(obsolete("-42949672k"), Some(Err(ParseError::Overflow)));
assert_eq!(
obsolete("-42949672960"),
obsolete_result(&["-n", "4294967295"])
);
assert_eq!(
obsolete("-42949672k"),
obsolete_result(&["-c", "4294967295"])
);
}
}
28 changes: 10 additions & 18 deletions tests/by-util/test_head.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,24 +321,20 @@ fn test_bad_utf8_lines() {
fn test_head_invalid_num() {
new_ucmd!()
.args(&["-c", "1024R", "emptyfile.txt"])
.fails()
.stderr_is(
"head: invalid number of bytes: '1024R': Value too large for defined data type\n",
);
.succeeds()
.no_output();
new_ucmd!()
.args(&["-n", "1024R", "emptyfile.txt"])
.fails()
.stderr_is(
"head: invalid number of lines: '1024R': Value too large for defined data type\n",
);
.succeeds()
.no_output();
new_ucmd!()
.args(&["-c", "1Y", "emptyfile.txt"])
.fails()
.stderr_is("head: invalid number of bytes: '1Y': Value too large for defined data type\n");
.succeeds()
.no_output();
new_ucmd!()
.args(&["-n", "1Y", "emptyfile.txt"])
.fails()
.stderr_is("head: invalid number of lines: '1Y': Value too large for defined data type\n");
.succeeds()
.no_output();
#[cfg(target_pointer_width = "32")]
{
let sizes = ["1000G", "10T"];
Expand All @@ -350,10 +346,7 @@ fn test_head_invalid_num() {
{
let sizes = ["-1000G", "-10T"];
for size in &sizes {
new_ucmd!()
.args(&["-c", size])
.fails()
.stderr_is("head: out of range integral type conversion attempted: number of -bytes or -lines is too large\n");
new_ucmd!().args(&["-c", size]).succeeds().no_output();
}
}
new_ucmd!()
Expand Down Expand Up @@ -778,8 +771,7 @@ fn test_value_too_large() {

new_ucmd!()
.args(&["-n", format!("{MAX}0").as_str(), "lorem_ipsum.txt"])
.fails()
.stderr_contains("Value too large for defined data type");
.succeeds();
}

#[test]
Expand Down
Loading