diff --git a/src/xlsx/mod.rs b/src/xlsx/mod.rs index bd3854ec..679cbb19 100644 --- a/src/xlsx/mod.rs +++ b/src/xlsx/mod.rs @@ -2788,20 +2788,10 @@ pub(crate) fn get_dimension(dimension: &[u8]) -> Result { start: parts[0], end: parts[0], }), - 2 => { - let rows = parts[1].0 - parts[0].0; - let columns = parts[1].1 - parts[0].1; - if rows > MAX_ROWS { - warn!("xlsx has more than maximum number of rows ({rows} > {MAX_ROWS})"); - } - if columns > MAX_COLUMNS { - warn!("xlsx has more than maximum number of columns ({columns} > {MAX_COLUMNS})"); - } - Ok(Dimensions { - start: parts[0], - end: parts[1], - }) - } + 2 => Ok(Dimensions { + start: parts[0], + end: parts[1], + }), len => Err(XlsxError::DimensionCount(len)), } } @@ -2821,6 +2811,13 @@ pub(crate) fn get_row(range: &[u8]) -> Result { get_row_and_optional_column(range).map(|(row, _)| row) } +/// Appends one base-26 letter to a 1-based column accumulator. +fn push_column_letter(col: u32, offset: u8) -> Result { + col.checked_mul(26) + .and_then(|col| col.checked_add(u32::from(offset) + 1)) + .ok_or(XlsxError::ColumnNumberOverflow) +} + /// Converts a text-based range name into its `(row, column)` position (0-based index). /// If the column component of the range is missing, a None is returned (for the column). /// If the row component of the range is missing, an Error is returned. @@ -2834,11 +2831,11 @@ fn get_row_and_optional_column(range: &[u8]) -> Result<(u32, Option), XlsxE let mut row: u32 = 0; while i < len { match range[i] { - c @ b'A'..=b'Z' => col = col * 26 + (c - b'A') as u32 + 1, - c @ b'a'..=b'z' => col = col * 26 + (c - b'a') as u32 + 1, + c @ b'A'..=b'Z' => col = push_column_letter(col, c - b'A')?, + c @ b'a'..=b'z' => col = push_column_letter(col, c - b'a')?, c @ b'0'..=b'9' => { // on first digit, capture it and transition to the row loop - row = (c - b'0') as u32; + row = u32::from(c - b'0'); i += 1; break; } @@ -2850,12 +2847,25 @@ fn get_row_and_optional_column(range: &[u8]) -> Result<(u32, Option), XlsxE // Row: accumulate base-10 from remaining digits (1-based in source) while i < len { match range[i] { - c @ b'0'..=b'9' => row = row * 10 + (c - b'0') as u32, + c @ b'0'..=b'9' => { + row = row + .checked_mul(10) + .and_then(|row| row.checked_add(u32::from(c - b'0'))) + .ok_or(XlsxError::RowNumberOverflow)?; + } c => return Err(XlsxError::Alphanumeric(c)), } i += 1; } + // Reject references outside the maximum grid size. + if row > MAX_ROWS { + return Err(XlsxError::RowNumberOverflow); + } + if col > MAX_COLUMNS { + return Err(XlsxError::ColumnNumberOverflow); + } + // Convert from 1-based to 0-based (col=0 means no column found) let row = row .checked_sub(1) @@ -4010,6 +4020,42 @@ mod tests { ); } + #[test] + fn test_reference_beyond_grid_is_an_error() { + // One past the grid on either axis is an error, and so is a + // reference that does not fit in u32 at all. + assert!(matches!( + get_row_column(b"XFE1"), + Err(XlsxError::ColumnNumberOverflow) + )); + assert!(matches!( + get_row_column(b"A1048577"), + Err(XlsxError::RowNumberOverflow) + )); + assert!(matches!( + get_row_column(b"ZZZZZZZ1"), + Err(XlsxError::ColumnNumberOverflow) + )); + assert!(matches!( + get_row_column(b"A99999999999"), + Err(XlsxError::RowNumberOverflow) + )); + assert!(matches!( + get_dimension(b"A1:ZZZZZZZ1"), + Err(XlsxError::ColumnNumberOverflow) + )); + assert!(matches!( + get_dimension(b"A1:A99999999999"), + Err(XlsxError::RowNumberOverflow) + )); + + // The bottom-right cell of the grid still parses. + assert_eq!( + get_row_column(b"XFD1048576").unwrap(), + (MAX_ROWS - 1, MAX_COLUMNS - 1) + ); + } + #[test] fn test_parse_error() { assert_eq!(