iframe-proxy

alexey-milovidov · 2026-03-20T07:54:53Z

The multiplication interval * scale_diff in ToStartOfInterval<Millisecond> and ToStartOfInterval<Microsecond> could overflow Int64 when the interval value is very large and the DateTime64 scale requires rescaling. Use mulOverflow to detect this and throw DECIMAL_OVERFLOW instead of triggering undefined behavior.

Found by AST fuzzer (UBSan): https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=100138&sha=f2962399a5dcac395a5540de8d06c3500e263ada&name_0=PR&name_1=AST%20fuzzer%20%28amd_ubsan%29
#100138

Changelog category (leave one):

Bug Fix (user-visible misbehavior in an official stable release)

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

Fix signed integer overflow (undefined behavior) in toStartOfInterval when called with very large millisecond/microsecond interval values on high-precision DateTime64.

Documentation entry for user-facing changes

Documentation is written (mandatory for new features)

Version info

Merged into: 26.4.1.737

…crosecond intervals The multiplication `milliseconds * scale_diff` (and similarly for microseconds) in `ToStartOfInterval` could overflow Int64 when the interval value is very large and the DateTime64 scale requires rescaling. Use `mulOverflow` to detect this and throw `DECIMAL_OVERFLOW` instead of triggering undefined behavior. Found by AST fuzzer (UBSan): https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=100138&sha=f2962399a5dcac395a5540de8d06c3500e263ada&name_0=PR&name_1=AST%20fuzzer%20%28amd_ubsan%29 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

clickhouse-gh · 2026-03-20T07:55:31Z

…visors The addition `res += static_cast<OriginResultType>(b)` in `PositiveModuloImpl` can overflow signed arithmetic when B is unsigned and b > INT64_MAX, because the cast to signed produces a negative value. Perform the addition in unsigned arithmetic instead, which is well-defined and produces the correct result since the mathematical value is always in [0, |b|). Caught by `FunctionsStress.stress` unit test under UBSan. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Add a stateless test that directly exercises the `positiveModulo` overflow edge cases with large unsigned divisors, as requested in the code review. The test covers the specific scenario where the intermediate addition `res + b` in `PositiveModuloImpl` would overflow signed Int64 arithmetic. #100156 (comment) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…Interval-overflow

alexey-milovidov · 2026-03-24T14:27:37Z

The AST fuzzer (arm_asan_ubsan) failure is unrelated to this PR — it's a pre-existing UBSan issue in BSINumericIndexedVector (issue #100052), already being fixed in #100086.

…Interval-overflow # Conflicts: # src/Functions/DivisionUtils.h

…Interval-overflow

clickhouse-gh · 2026-03-29T01:29:39Z

+                throw DB::Exception(ErrorCodes::DECIMAL_OVERFLOW, "Numeric overflow");
            if (t >= 0) [[likely]] /// When we divide the `t` value we should round the result
-                return (t + scale_diff / 2) / (microseconds * scale_diff) * microseconds;
+                return (t + scale_diff / 2) / microseconds_scaled * microseconds;


mulOverflow covers microseconds * scale_diff, but the subsequent rounding expression still has signed-overflow UB for large positive DateTime64 values:

(t + scale_diff / 2) / microseconds_scaled * microseconds

Concrete trace for scale_multiplier = 10^9 (so scale_diff = 1000 in the microsecond branch) and
t = toDateTime64('2262-04-11 23:47:16.854775807', 9) (t == 9223372036854775807):

scale_diff / 2 == 500

t + 500 overflows Int64 before the division

So this PR still leaves UB reachable in the same code path it is fixing. Please guard the addition with common::addOverflow (or equivalent) and throw DECIMAL_OVERFLOW on overflow.

The same pattern is also present in the millisecond specialization.

clickhouse-gh · 2026-03-29T01:29:52Z

+                throw DB::Exception(ErrorCodes::DECIMAL_OVERFLOW, "Numeric overflow");
            if (t >= 0) [[likely]]  /// When we divide the `t` value we should round the result
-                return (t + scale_diff / 2) / (milliseconds * scale_diff) * milliseconds;
+                return (t + scale_diff / 2) / milliseconds_scaled * milliseconds;


Same overflow pattern exists in the millisecond specialization:

(t + scale_diff / 2) / milliseconds_scaled * milliseconds

For large positive t close to INT64_MAX and scale_multiplier > 1000, t + scale_diff / 2 can overflow before division. Please apply the same common::addOverflow guard here and throw DECIMAL_OVERFLOW on overflow.

…Interval-overflow # Conflicts: # src/Functions/DateTimeTransforms.h

Use already-computed `microseconds_scaled` / `milliseconds_scaled` instead of recomputing the same product into a `divisor` variable inside the block. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…Interval-overflow

Use a negative `DateTime64` (pre-1970) in the `toStartOfInterval` overflow test so it exercises the `else` branch where master lacked the `mulOverflow` check. The previous test used a positive timestamp which hit the `t >= 0` branch where the overflow check already existed on master, causing bugfix validation to fail (test passed on both old and new code). Remove the `positiveModulo` overflow test since the underlying fix was already merged to master via a separate PR, making this test irrelevant for bugfix validation of this PR. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…Interval-overflow

alexey-milovidov · 2026-04-07T00:26:22Z

The Stress test (arm_msan) failure is fixed by #101239, which should be merged first. After it is merged, please update the branch to include the fix.

clickhouse-gh · 2026-04-09T02:13:45Z

LLVM Coverage Report

Metric	Baseline	Current	Δ
Lines	84.00%	84.00%	+0.00%
Functions	90.90%	90.90%	+0.00%
Branches	76.50%	76.50%	+0.00%

Changed lines: 100.00% (30/30) | lost baseline coverage: 32 line(s) · Uncovered code

Full report · Diff report

clickgapai · 2026-04-10T00:41:27Z

clickhouse-gh Bot added the pr-bugfix Pull request with bugfix, not backported by default label Mar 20, 2026

alexey-milovidov and others added 4 commits March 21, 2026 10:47

Merge branch 'master' into fix-ubsan-toStartOfInterval-overflow

b244732

Merge master into fix-ubsan-toStartOfInterval-overflow

ab41cb7

Merge master into fix-ubsan-toStartOfInterval-overflow

d1f6911

clickhouse-gh Bot reviewed Mar 23, 2026

View reviewed changes

Comment thread src/Functions/DivisionUtils.h Outdated

alexey-milovidov and others added 3 commits March 23, 2026 13:20

Merge branch 'master' into fix-ubsan-toStartOfInterval-overflow

c77ced7

Merge remote-tracking branch 'origin/master' into fix-ubsan-toStartOf…

148f188

…Interval-overflow

groeneai mentioned this pull request Mar 25, 2026

Fix signed integer overflow in toStartOfInterval for Week, Quarter, and Year intervals #100745

Closed

1 task

alexey-milovidov added 3 commits March 26, 2026 23:15

Merge remote-tracking branch 'origin/master' into fix-ubsan-toStartOf…

235fcb4

…Interval-overflow # Conflicts: # src/Functions/DivisionUtils.h

Merge branch 'master' into fix-ubsan-toStartOfInterval-overflow

68342ec

Merge remote-tracking branch 'origin/master' into fix-ubsan-toStartOf…

a35da10

…Interval-overflow

clickhouse-gh Bot reviewed Mar 29, 2026

View reviewed changes

alexey-milovidov and others added 6 commits March 30, 2026 03:48

Merge remote-tracking branch 'origin/master' into fix-ubsan-toStartOf…

ad8adaf

…Interval-overflow # Conflicts: # src/Functions/DateTimeTransforms.h

Remove redundant mulOverflow recomputation after merge

39f5b71

Use already-computed `microseconds_scaled` / `milliseconds_scaled` instead of recomputing the same product into a `divisor` variable inside the block. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Merge remote-tracking branch 'origin/master' into fix-ubsan-toStartOf…

eeb602b

…Interval-overflow

Merge remote-tracking branch 'origin/master' into fix-ubsan-toStartOf…

1279622

…Interval-overflow

Merge remote-tracking branch 'origin/master' into fix-ubsan-toStartOf…

5ed00e0

…Interval-overflow

nickitat approved these changes Mar 30, 2026

View reviewed changes

clickhouse-gh Bot assigned nickitat Mar 30, 2026

Merge branch 'master' into fix-ubsan-toStartOfInterval-overflow

95259a2

alexey-milovidov added 2 commits April 6, 2026 17:48

Merge branch 'master' into fix-ubsan-toStartOfInterval-overflow

734c979

Merge branch 'master' into fix-ubsan-toStartOfInterval-overflow

9d6edd6

alexey-milovidov merged commit e6b1dea into master Apr 9, 2026
163 of 164 checks passed

alexey-milovidov deleted the fix-ubsan-toStartOfInterval-overflow branch April 9, 2026 19:27

robot-ch-test-poll3 added the pr-synced-to-cloud The PR is synced to the cloud repo label Apr 9, 2026

Sunbelt Computer Software

PL/B Language Development and Support

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix signed integer overflow in toStartOfInterval for Millisecond/Microsecond intervals#100156

Fix signed integer overflow in toStartOfInterval for Millisecond/Microsecond intervals#100156
alexey-milovidov merged 20 commits into
masterfrom
fix-ubsan-toStartOfInterval-overflow

alexey-milovidov commented Mar 20, 2026 •

edited by robot-clickhouse

Loading

Uh oh!

clickhouse-gh Bot commented Mar 20, 2026 •

edited

Loading

Uh oh!

Uh oh!

alexey-milovidov commented Mar 24, 2026

Uh oh!

clickhouse-gh Bot Mar 29, 2026

Uh oh!

clickhouse-gh Bot Mar 29, 2026

Uh oh!

alexey-milovidov commented Apr 7, 2026

Uh oh!

clickhouse-gh Bot commented Apr 9, 2026

Uh oh!

Uh oh!

clickgapai commented Apr 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Sunbelt Computer Software

PL/B Language Development and Support

Uh oh!

Conversation

alexey-milovidov commented Mar 20, 2026 • edited by robot-clickhouse Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changelog category (leave one):

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

Documentation entry for user-facing changes

Version info

Uh oh!

clickhouse-gh Bot commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

AI Review

Summary

Missing context

ClickHouse Rules

Final Verdict

Uh oh!

Uh oh!

alexey-milovidov commented Mar 24, 2026

Uh oh!

clickhouse-gh Bot Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

clickhouse-gh Bot Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

alexey-milovidov commented Apr 7, 2026

Uh oh!

clickhouse-gh Bot commented Apr 9, 2026

LLVM Coverage Report

Uh oh!

Uh oh!

clickgapai commented Apr 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

alexey-milovidov commented Mar 20, 2026 •

edited by robot-clickhouse

Loading

clickhouse-gh Bot commented Mar 20, 2026 •

edited

Loading