microsoft / microsoft/mssql-rs

SQLGetData/SQLFetch of an out-of-range time silently truncates the hour and returns SQL_SUCCESS

Open Beginner friendly
#526 0 comments 0 reactions 0 assignees View on GitHub
bug
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.