Skip to content

Commit 6effa29

Browse files
authored
Fix incorrect bitwise comparison (#226)
* Fix incorrect bitwise comparison _getMessageAttestationsStorage()[digest].attestedTransceivers & uint64(1 << index) == 1; The call at the beginning is getting a bitmap of attested transceivers. So, 7 (b111) would be 3 accepted transceivers. Next, we are computing the index to compare on the bitmap with transceiver index. So, if we want check the second index then 1 << 1 would work to produce 2 or the middle bit out of the 3 in the example. Finally, we're comparing this with 1, which is where the bug is at. This is ONLY true if we're checking the first index. For instance, if we're checking the second index (value of 1) then we'd do 7 & (1 << 1) == 1 or 7 & 2 == 1 which is 2 == 1 . So, the comparison will fail on all indexes that aren't the first one (value of 0). So, the fix is to check if the value is greater than 0. This means that if the singular bit is set, then it's already been attested to * Fix tests to use the proper error message on replay checks * Fix format for CI
1 parent 22bde0c commit 6effa29

File tree

3 files changed

+17
-21
lines changed

3 files changed

+17
-21
lines changed

evm/src/NttManager/NttManagerState.sol

+1-1
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ abstract contract NttManagerState is
170170
/// @inheritdoc INttManagerState
171171
function transceiverAttestedToMessage(bytes32 digest, uint8 index) public view returns (bool) {
172172
return
173-
_getMessageAttestationsStorage()[digest].attestedTransceivers & uint64(1 << index) == 1;
173+
_getMessageAttestationsStorage()[digest].attestedTransceivers & uint64(1 << index) > 0;
174174
}
175175

176176
/// @inheritdoc INttManagerState

evm/test/NttManager.t.sol

+7-13
Original file line numberDiff line numberDiff line change
@@ -453,23 +453,17 @@ contract TestNttManager is Test, INttManagerEvents, IRateLimiterEvents {
453453
assertEq(token.balanceOf(address(user_B)), transferAmount.untrim(token.decimals()));
454454
}
455455

456-
// replay protection
456+
// replay protection for transceiver
457457
vm.recordLogs();
458-
e2.receiveMessage(encodedEm);
459-
460-
{
461-
Vm.Log[] memory entries = vm.getRecordedLogs();
462-
assertEq(entries.length, 2);
463-
assertEq(entries[1].topics.length, 3);
464-
assertEq(entries[1].topics[0], keccak256("MessageAlreadyExecuted(bytes32,bytes32)"));
465-
assertEq(entries[1].topics[1], toWormholeFormat(address(nttManager)));
466-
assertEq(
467-
entries[1].topics[2],
458+
vm.expectRevert(
459+
abi.encodeWithSignature(
460+
"TransceiverAlreadyAttestedToMessage(bytes32)",
468461
TransceiverStructs.nttManagerMessageDigest(
469462
TransceiverHelpersLib.SENDING_CHAIN_ID, m
470463
)
471-
);
472-
}
464+
)
465+
);
466+
e2.receiveMessage(encodedEm);
473467
}
474468

475469
// TODO:

evm/test/RateLimit.t.sol

+9-7
Original file line numberDiff line numberDiff line change
@@ -559,18 +559,20 @@ contract TestRateLimit is Test, IRateLimiterEvents {
559559
// assert user now has funds
560560
assertEq(token.balanceOf(address(user_B)), 50 * 10 ** (token.decimals() - 8));
561561

562-
// replay protection
562+
// replay protection on executeMsg
563563
vm.recordLogs();
564-
e2.receiveMessage(encodedEm);
564+
nttManager.executeMsg(
565+
TransceiverHelpersLib.SENDING_CHAIN_ID, toWormholeFormat(address(nttManager)), m
566+
);
565567

566568
{
567569
Vm.Log[] memory entries = vm.getRecordedLogs();
568-
assertEq(entries.length, 2);
569-
assertEq(entries[1].topics.length, 3);
570-
assertEq(entries[1].topics[0], keccak256("MessageAlreadyExecuted(bytes32,bytes32)"));
571-
assertEq(entries[1].topics[1], toWormholeFormat(address(nttManager)));
570+
assertEq(entries.length, 1);
571+
assertEq(entries[0].topics.length, 3);
572+
assertEq(entries[0].topics[0], keccak256("MessageAlreadyExecuted(bytes32,bytes32)"));
573+
assertEq(entries[0].topics[1], toWormholeFormat(address(nttManager)));
572574
assertEq(
573-
entries[1].topics[2],
575+
entries[0].topics[2],
574576
TransceiverStructs.nttManagerMessageDigest(
575577
TransceiverHelpersLib.SENDING_CHAIN_ID, m
576578
)

0 commit comments

Comments
 (0)