Avoid returning a BIP9Stats object with uninitialized values #10957

pull practicalswift wants to merge 1 commits into bitcoin:master from practicalswift:bip9status changing 1 files +1 −1

practicalswift commented at 9:41 PM on July 30, 2017: contributor

Uninitialized data potentially used in rpc/blockchain.cpp:

static UniValue BIP9SoftForkDesc(const Consensus::Params& consensusParams, Consensus::DeploymentPos id)
{
    ...
    const ThresholdState thresholdState = VersionBitsTipState(consensusParams, id);
    ...
    if (THRESHOLD_STARTED == thresholdState)
    {
        UniValue statsUV(UniValue::VOBJ);
        BIP9Stats statsStruct = VersionBitsTipStatistics(consensusParams, id);
        statsUV.push_back(Pair("period", statsStruct.period));
        statsUV.push_back(Pair("threshold", statsStruct.threshold));
        statsUV.push_back(Pair("elapsed", statsStruct.elapsed));
        statsUV.push_back(Pair("count", statsStruct.count));
        statsUV.push_back(Pair("possible", statsStruct.possible));
        rv.push_back(Pair("statistics", statsUV));
    }
    ...
    return rv;
}

Friendly ping @pinheadmz :-)

practicalswift renamed this:
~~Do not return a BIP9Stats object with uninitialized values~~
Avoid returning a BIP9Stats object with uninitialized values
on Jul 30, 2017
practicalswift force-pushed on Jul 30, 2017
sipa commented at 4:30 AM on July 31, 2017: member

utACK bf2f4c36c8e37a29f81833505f76a99c5faa8a79
pinheadmz commented at 4:38 AM on July 31, 2017: member

utACK thanks! I think this would only apply to the genesis block, right? Which would never be THRESHOLD_STARTED... I don't know if that pindex==NULL code will ever run actually.

in src/versionbits.cpp:116 in bf2f4c36c8 outdated

 111 | @@ -112,8 +112,12 @@ BIP9Stats AbstractThresholdConditionChecker::GetStateStatisticsFor(const CBlockI
 112 |      stats.period = Period(params);
 113 |      stats.threshold = Threshold(params);
 114 |  
 115 | -    if (pindex == NULL)
 116 | +    if (pindex == NULL) {
 117 | +        stats.elapsed = 0;

promag commented at 2:58 PM on July 31, 2017:

I see no problem in:

moving all default values to BIP9Stats constructor, or
moving these up even if they are overwritten below.

promag commented at 3:00 PM on July 31, 2017: member

utACK bf2f4c3.
TheBlueMatt commented at 7:26 PM on August 1, 2017: member

Or maybe just initialize the stats object with an = {}?

Avoid returning a BIP9Stats object with uninitialized values

Uninitialized data potentially used in `rpc/blockchain.cpp`:

```
static UniValue BIP9SoftForkDesc(const Consensus::Params& consensusParams, Consensus::DeploymentPos id)
{
    ...
    const ThresholdState thresholdState = VersionBitsTipState(consensusParams, id);
    ...
    if (THRESHOLD_STARTED == thresholdState)
    {
        UniValue statsUV(UniValue::VOBJ);
        BIP9Stats statsStruct = VersionBitsTipStatistics(consensusParams, id);
        statsUV.push_back(Pair("period", statsStruct.period));
        statsUV.push_back(Pair("threshold", statsStruct.threshold));
        statsUV.push_back(Pair("elapsed", statsStruct.elapsed));
        statsUV.push_back(Pair("count", statsStruct.count));
        statsUV.push_back(Pair("possible", statsStruct.possible));
        rv.push_back(Pair("statistics", statsUV));
    }
    ...
    return rv;
}
```

3eb53b8671

practicalswift force-pushed on Aug 1, 2017
practicalswift commented at 9:01 PM on August 1, 2017: contributor

@TheBlueMatt Thanks for the review! Fixed! :-)
jonasschnelli added the label Refactoring on Aug 3, 2017
jonasschnelli commented at 9:53 AM on August 3, 2017: contributor

utACK 3eb53b867153c957529484b5338d27e69de027c1
MarcoFalke commented at 11:55 AM on August 3, 2017: member

utACK 3eb53b8
TheBlueMatt commented at 1:55 PM on August 3, 2017: member

utACK
fanquake commented at 1:37 PM on August 12, 2017: member

trivialACK 3eb53b8
MarcoFalke merged this on Aug 16, 2017
MarcoFalke closed this on Aug 16, 2017
MarcoFalke referenced this in commit a46a671e25 on Aug 16, 2017
MarcoFalke referenced this in commit 50bd3f626d on Oct 3, 2017
PastaPastaPasta referenced this in commit 722dc723ea on Sep 19, 2019
PastaPastaPasta referenced this in commit e5c938b3cb on Sep 23, 2019
PastaPastaPasta referenced this in commit 443c4c50f5 on Sep 24, 2019
codablock referenced this in commit 9a5b798e9c on Sep 25, 2019
barrystyle referenced this in commit eed4ee6e75 on Jan 22, 2020
practicalswift deleted the branch on Apr 10, 2021
DrahtBot locked this on Aug 16, 2022

Contributors

Labels