Fix Various Instances of Undefined Behaviour #1198
Conversation
|
This pull request fixes 4 alerts when merging cf2b38b into 1e9bad8 - view on LGTM.com fixed alerts:
|
ghost
changed the title
|
This pull request fixes 2 alerts when merging 3546005 into 1e9bad8 - view on LGTM.com fixed alerts:
|
ghost
force-pushed
the
quorum-ub-fix
branch
from
3546005 to
46f59d7
Compare
ghost
force-pushed
the
quorum-ub-fix
branch
from
46f59d7 to
d8b701c
Compare
src/llmq/quorums_blockprocessor.cpp
Outdated
| @@ -321,7 +321,8 @@ bool CQuorumBlockProcessor::GetCommitmentsFromBlock(const CBlock& block, const C | |||
| return state.DoS(100, false, REJECT_INVALID, "bad-qc-dup"); | |||
| } | |||
|
|
|||
| ret.emplace((Consensus::LLMQType)qc.commitment.llmqType, std::move(qc.commitment)); | |||
| auto temp = (Consensus::LLMQType) qc.commitment.llmqType; | |||
There was a problem hiding this comment.
don't name variable temp please. llmqType is much better name
There was a problem hiding this comment.
Use of the name temp is an indicator that the variable is semantically required. Can document in a comment instead if that's preferable.
|
This pull request introduces 2 alerts when merging 40ce1d2 into bd9190d - view on LGTM.com new alerts:
Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog. |
* Fix UB in bls. * Fix UB in CDKGSessionHandler::SleepBeforePhase. * Fix UB in CQuorumBlockProcessor::GetCommitmentsFromBlock.
PR intention
This fixes several instances of UB.
Code changes brief
CQuorumBlockProcessor::GetCommitmentsFromBlock- std::move() was used incorrectly, leading to a use after free issue.CDKGSessionHandler::SleepBeforePhase- An invalid implicit cast fromUINT64_MAXtodoubleis performed in some cases.