Page MenuHomePhabricator
Differential D6025

Fix potential timedata overflow
ClosedPublic
Actions

Authored by jasonbcox on May 11 2020, 03:04.

Details

Reviewers
Fabien
deadalnix
Group Reviewers
Restricted Project
Commits
rSTAGING59dff0e81865: Fix potential timedata overflow
rABC59dff0e81865: Fix potential timedata overflow
Summary

The add before division can potentially cause an overflow.
If we divide before adding, no overflow can occur.

Test Plan

ninja check

Diff Detail

Repository
rABC Bitcoin ABC
Branch
vsorted-overflow
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 10698
Build 19192: Default Diff Build & Tests
Build 19191: arc lint + arc unit

Event Timeline

jasonbcox created this revision.May 11 2020, 03:04
Herald added a reviewer: Restricted Project. · View Herald TranscriptMay 11 2020, 03:04
jasonbcox requested review of this revision.May 11 2020, 03:04
teamcity added a comment.May 11 2020, 03:05

Build Bitcoin-ABC / Diffs / Diff Testing (build-diff) started.

teamcity added a comment.May 11 2020, 03:15

Build Bitcoin-ABC / Diffs / Diff Testing (build-diff) passed.

Harbormaster completed remote builds in B10681: Diff 19879.May 11 2020, 03:15
Fabien requested changes to this revision.May 11 2020, 08:06
Fabien added a subscriber: Fabien.
Fabien added inline comments.
src/timedata.h
54 ↗(On Diff #19879)

Let's say that vSorted[vSortedSize / 2 - 1] == vSorted[vSortedSize / 2 ] == 3.
Before the result was (3 + 3) / 2 = 6/2 = 3 and now it is 3/2 + 3/2 = 1 + 1 = 2 because of integer division.

This revision now requires changes to proceed.May 11 2020, 08:06
jasonbcox requested review of this revision.May 11 2020, 16:23
jasonbcox added inline comments.
src/timedata.h
54 ↗(On Diff #19879)

Although there is a slight change in behavior when both elements are odd (median is 1 less), this result is inconsequential since the time offsets are not super accurate anyway.

deadalnix requested changes to this revision.May 11 2020, 18:01
deadalnix added a subscriber: deadalnix.
deadalnix added inline comments.
src/timedata.h
54 ↗(On Diff #19879)

int a, b;
int avg = a / 2 + b / 2 + (a & b & 1);

voila.

This revision now requires changes to proceed.May 11 2020, 18:01
jasonbcox updated this revision to Diff 19915.May 11 2020, 18:30

Fix median calculation when both elements are odd

teamcity added a comment.May 11 2020, 18:31

Build Bitcoin-ABC / Diffs / Diff Testing (build-diff) started.

teamcity added a comment.May 11 2020, 18:41

Build Bitcoin-ABC / Diffs / Diff Testing (build-diff) passed.

Harbormaster completed remote builds in B10698: Diff 19915.May 11 2020, 18:41
deadalnix accepted this revision.May 11 2020, 19:00
Fabien accepted this revision.May 11 2020, 22:42
This revision is now accepted and ready to land.May 11 2020, 22:42
Closed by commit rABC59dff0e81865: Fix potential timedata overflow (authored by jasonbcox). · Explain WhyMay 12 2020, 00:14
This revision was automatically updated to reflect the committed changes.
jasonbcox added a commit: rABC59dff0e81865: Fix potential timedata overflow.
jasonbcox added a commit: rSTAGING59dff0e81865: Fix potential timedata overflow.May 12 2020, 00:20

Revision Contents
Changeset List

PathSize
src/
5 lines

Diff 19915

View Options

src/timedata.h

Loading...