microsoft / microsoft/mssql-rs
SQLGetData/SQLFetch of an out-of-range time silently truncates the hour and returns SQL_SUCCESS
- Dominant language
- Rust
- Stars
- 53
- Forks
- 14
- Avg merge
- 1d 15h
- Merged PRs (30d)
- 137
Description
### Describe the bug
`hms_from_ticks_100ns` (`mssql-odbc/src/conversion/datetime.rs:161`) computes the clock fields from a 100 ns tick count with no range check:
```rust
TimeOfDay {
hour: (secs / 3600) as u16,
minute: ((secs % 3600) / 60) as u16,
second: (secs % 60) as u16,
fraction_ns,
}
```
`secs / 3600` is truncated with `as u16` instead of validated. A valid time-of-day is `< 24 * 3600 * 10_000_000` ticks; any larger `time_nanoseconds` (the decoder's name for the 100 ns tick field) produces an `hour` outside `0..=23`, silently wrapped into `0..=65535`, and the conversion still reports `Ok(ConvOk::Exact)` → `SQL_SUCCESS`.
The callers `time_parts` and `datetime2_parts` (`mssql-odbc/src/conversion/fetch_convert.rs`) don't range-check the ticks either, so a corrupt or hostile server row that decodes to an out-of-range time is handed back through `SQLGetData` / block fetch as a successful conversion with a wrong value. This is the value-correctness sibling of the datetimeoffset overflow fixed in #511 — that path had an `i64` add that panicked; these paths have no add, so they truncate instead of panicking, which is why a fuzzer that only watches for panics can't surface it.
### Steps to reproduce
Convert a `DateTime2` (or `Time`) column whose tick field is outside a single day into `SQL_C_TYPE_TIMESTAMP` (or `SQL_C_TYPE_TIME`). Throwaway unit test that demonstrates it:
```rust
let dt2 = ColumnValues::DateTime2(SqlDateTime2 {
days: 0,
time: SqlTime { time_nanoseconds: i64::MAX as u64, scale: 7 },
});
// convert dt2 to SQL_C_TYPE_TIMESTAMP
```
### Expected behavior
An out-of-range tick count is rejected as a conversion error (e.g. ODBC `22007` / a `None` → normal ODBC conversion error), the same way `datetimeoffset_parts` now rejects out-of-range days via the `0..=MAX_DAYS_SINCE_0001` check — never a wrong value under `SQL_SUCCESS`.
### Actual behavior
Returns `Ok(ConvOk::Exact)` / `SQL_SUCCESS` with a truncated, out-of-range field, e.g.:
```
SqlTimestampStruct { year: 1, month: 1, day: 1, hour: 24554, minute: 48, second: 5, fraction: 477580700 }
```
`hour: 24554` is handed back in a `SQLUSMALLINT` under a success return.
### Version
commit 4ca532ae (branch `dev/saurabh/odbc-fuzz-testing`)
### Affected crate
mssql-odbc
### Environment
- OS: Ubuntu 22.04 (also platform-independent)
- Not tied to a specific SQL Server version — reproducible from any row that decodes to an out-of-range time tick count.
### Additional context
Surfaced while reviewing the ODBC fuzz PR #511. It is pre-existing (not introduced by that PR) and out of scope for it, so it is filed separately. A range check belongs in `hms_from_ticks_100ns` or in each caller (`time_parts`, `datetime2_parts`) before the fields are trusted.
Contributor guide
Research direction
Start in mssql-odbc/src/conversion/datetime.rs at hms_from_ticks_100ns, then inspect time_parts and datetime2_parts in mssql-odbc/src/conversion/fetch_convert.rs. Reproduce the DateTime2 example from the issue and add coverage for an out-of-range tick field. Done means SQLGetData and block fetch return a normal conversion error rather than SQL_SUCCESS with an invalid hour.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 86/100