Skip to content

Commit

Permalink
Merge pull request #23 from spherex-collab/L11
Browse files Browse the repository at this point in the history 
L11
  • Loading branch information
@sfeyal
sfeyal committed Jul 31, 2023
2 parents ed86e8a + 950a31d commit b9e1c2b
Show file tree
Hide file tree
Showing 7 changed files with 102 additions and 87 deletions.
29 changes: 17 additions & 12 deletions gas-report
View file
Original file line number Diff line number Diff line change
@@ -1,16 +1,21 @@
| src/SphereXEngine.sol:SphereXEngine contract | | | | | |
|----------------------------------------------|-----------------|-------|--------|-------|---------|
| Deployment Cost | Deployment Size | | | | |
| 740880 | 3366 | | | | |
| 1847252 | 10391 | | | | |
| Function Name | min | avg | median | max | # calls |
| activateRules | 657 | 3073 | 803 | 5603 | 95 |
| addAllowedPatterns | 3266 | 22313 | 23166 | 45568 | 123 |
| addAllowedSender | 23220 | 23220 | 23220 | 23220 | 88 |
| deactivateRules | 508 | 2082 | 2607 | 2607 | 4 |
| removeAllowedPatterns | 943 | 1210 | 1344 | 1344 | 3 |
| removeAllowedSender | 2787 | 4217 | 4932 | 4932 | 3 |
| sphereXValidateInternalPost | 2045 | 2045 | 2045 | 2045 | 8 |
| sphereXValidateInternalPre | 457 | 5041 | 2063 | 18952 | 157 |
| sphereXValidatePost | 1057 | 2880 | 2736 | 4616 | 86 |
| sphereXValidatePre | 966 | 9340 | 8167 | 19456 | 94 |
| transferOwnership | 7343 | 7343 | 7343 | 7343 | 1 |
| OPERATOR_ROLE | 338 | 338 | 338 | 338 | 3 |
| acceptDefaultAdminTransfer | 27892 | 27892 | 27892 | 27892 | 1 |
| addAllowedPatterns | 5406 | 24458 | 25306 | 48304 | 123 |
| addAllowedSender | 1007 | 24848 | 25351 | 25351 | 93 |
| beginDefaultAdminTransfer | 12095 | 12095 | 12095 | 12095 | 1 |
| configureRules | 2352 | 22409 | 24252 | 26252 | 96 |
| deactivateAllRules | 2705 | 5308 | 6176 | 6176 | 4 |
| grantRole | 29689 | 31303 | 29689 | 34533 | 3 |
| removeAllowedPatterns | 2777 | 3363 | 3656 | 3656 | 3 |
| removeAllowedSender | 3006 | 5402 | 6600 | 6600 | 3 |
| revokeRole | 2948 | 2948 | 2948 | 2948 | 1 |
| sphereXValidateInternalPost | 458 | 2129 | 1742 | 3677 | 79 |
| sphereXValidateInternalPre | 434 | 4161 | 1962 | 8847 | 86 |
| sphereXValidatePost | 992 | 3147 | 2659 | 4659 | 86 |
| sphereXValidatePre | 926 | 5823 | 2449 | 11334 | 94 |
| supportsInterface | 435 | 435 | 435 | 435 | 49 |
89 changes: 46 additions & 43 deletions src/SphereXEngine.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,29 +3,34 @@

pragma solidity ^0.8.17;

import {AccessControlDefaultAdminRules, IERC165} from "openzeppelin-contracts/access/AccessControlDefaultAdminRules.sol";
import {
AccessControlDefaultAdminRules, IERC165
} from "openzeppelin-contracts/access/AccessControlDefaultAdminRules.sol";
import {ISphereXEngine} from "./ISphereXEngine.sol";

/**
* @title SphereX Engine
* @notice Gathers information about an ongoing transaction and reverts if it seems malicious
*/
contract SphereXEngine is ISphereXEngine, AccessControlDefaultAdminRules {
bytes8 private _engineRules; // By default the contract will be deployed with no guarding rules activated
mapping(address => bool) private _allowedSenders;
mapping(uint256 => bool) private _allowedPatterns;
struct FlowConfiguration {
uint16 depth;
// Represent bytes3(keccak256(abi.encode(block.number, tx.origin, block.difficulty, block.timestamp)))
bytes3 txBoundaryHash;
uint216 pattern;
}

// We initialize the next variables to 1 and not 0 to save gas costs on future transactions
uint256 private _currentPattern = PATTERN_START;
uint256 private _callDepth = DEPTH_START;
bytes8 internal _engineRules; // By default the contract will be deployed with no guarding rules activated
mapping(address => bool) internal _allowedSenders;
mapping(uint216 => bool) internal _allowedPatterns;

// Represent keccak256(abi.encode(block.number, tx.origin))
bytes32 private _lastTxBoundaryHash = bytes32(uint256(1));
FlowConfiguration internal _flowConfig = FlowConfiguration(DEPTH_START, bytes3(uint24(1)), PATTERN_START);

uint256 private constant PATTERN_START = 1;
uint256 private constant DEPTH_START = 1;
bytes32 private constant DEACTIVATED = bytes32(0);
uint64 private constant RULES_1_AND_2_TOGETHER = 3;
// We initialize the next variables to 1 and not 0 to save gas costs on future transactions
uint216 internal constant PATTERN_START = 1;
uint16 internal constant DEPTH_START = 1;
bytes32 internal constant DEACTIVATED = bytes32(0);
uint64 internal constant RULES_1_AND_2_TOGETHER = 3;

bytes32 public constant OPERATOR_ROLE = keccak256("OPERATOR_ROLE");

Expand All @@ -42,8 +47,8 @@ contract SphereXEngine is ISphereXEngine, AccessControlDefaultAdminRules {
event ConfigureRules(bytes8 oldRules, bytes8 newRules);
event AddedAllowedSenders(address[] senders);
event RemovedAllowedSenders(address[] senders);
event AddedAllowedPatterns(uint256[] patterns);
event RemovedAllowedPatterns(uint256[] patterns);
event AddedAllowedPatterns(uint216[] patterns);
event RemovedAllowedPatterns(uint216[] patterns);

modifier returnsIfNotActivated() {
if (_engineRules == DEACTIVATED) {
Expand Down Expand Up @@ -118,7 +123,7 @@ contract SphereXEngine is ISphereXEngine, AccessControlDefaultAdminRules {
* Add allowed patterns - these are representation of allowed flows of transactions, and prefixes of these flows
* @param patterns list of flows to allow as valid and non-malicious flows
*/
function addAllowedPatterns(uint256[] calldata patterns) external onlyOperator {
function addAllowedPatterns(uint216[] calldata patterns) external onlyOperator {
for (uint256 i = 0; i < patterns.length; ++i) {
_allowedPatterns[patterns[i]] = true;
}
Expand All @@ -130,7 +135,7 @@ contract SphereXEngine is ISphereXEngine, AccessControlDefaultAdminRules {
* that are no longer considered valid and benign
* @param patterns list of flows that no longer considered valid and non-malicious
*/
function removeAllowedPatterns(uint256[] calldata patterns) external onlyOperator {
function removeAllowedPatterns(uint216[] calldata patterns) external onlyOperator {
for (uint256 i = 0; i < patterns.length; ++i) {
_allowedPatterns[patterns[i]] = false;
}
Expand All @@ -142,38 +147,37 @@ contract SphereXEngine is ISphereXEngine, AccessControlDefaultAdminRules {
/**
* Checks if rule1 is activated.
*/
function _isRule1Activated() private view returns (bool) {
function _isRule1Activated() internal view returns (bool) {
return (_engineRules & bytes8(uint64(1))) > 0;
}

/**
* update the current CF pattern with a new positive number (signifying function entry),
* @param num element to add to the flow.
*/
function _addCfElementFunctionEntry(int256 num) private {
function _addCfElementFunctionEntry(int256 num) internal {
require(num > 0, "SphereX error: expected positive num");
uint256 callDepth = _callDepth;
uint256 currentPattern = _currentPattern;
FlowConfiguration memory flowConfig = _flowConfig;

// Upon entry to a new function we should check if we are at the same transaction
// or a new one. in case of a new one we need to reinit the currentPattern, and save
// the new transaction "boundry" (block.number+tx.origin+block.timestamp+block.difficulty)
bytes32 currentTxBoundryHash = keccak256(abi.encode(block.number, tx.origin, block.timestamp, block.difficulty));
if (currentTxBoundryHash != _lastTxBoundaryHash) {
currentPattern = PATTERN_START;
_lastTxBoundaryHash = currentTxBoundryHash;
if (callDepth != DEPTH_START) {
bytes3 currentTxBoundaryHash =
bytes3(keccak256(abi.encode(block.number, tx.origin, block.timestamp, block.difficulty)));
if (currentTxBoundaryHash != flowConfig.txBoundaryHash) {
flowConfig.pattern = PATTERN_START;
flowConfig.txBoundaryHash = currentTxBoundaryHash;
if (flowConfig.depth != DEPTH_START) {
// This is an edge case we (and the client) should be able to monitor easily.
emit TxStartedAtIrregularDepth();
callDepth = DEPTH_START;
flowConfig.depth = DEPTH_START;
}
}

currentPattern = uint256(keccak256(abi.encode(num, currentPattern)));
++callDepth;
flowConfig.pattern = uint216(bytes27(keccak256(abi.encode(num, flowConfig.pattern))));
++flowConfig.depth;

_callDepth = callDepth;
_currentPattern = currentPattern;
_flowConfig = flowConfig;
}

/**
Expand All @@ -182,32 +186,31 @@ contract SphereXEngine is ISphereXEngine, AccessControlDefaultAdminRules {
* @param num element to add to the flow. should be negative.
* @param forceCheck force the check of the current pattern, even if normal test conditions don't exist.
*/
function _addCfElementFunctionExit(int256 num, bool forceCheck) private {
function _addCfElementFunctionExit(int256 num, bool forceCheck) internal {
require(num < 0, "SphereX error: expected negative num");
uint256 callDepth = _callDepth;
uint256 currentPattern = _currentPattern;
FlowConfiguration memory flowConfig = _flowConfig;


currentPattern = uint256(keccak256(abi.encode(num, currentPattern)));
--callDepth;
flowConfig.pattern = uint216(bytes27(keccak256(abi.encode(num, flowConfig.pattern))));
--flowConfig.depth;

if ((forceCheck) || (callDepth == DEPTH_START)) {
_checkCallFlow(currentPattern);
if ((forceCheck) || (flowConfig.depth == DEPTH_START)) {
_checkCallFlow(flowConfig.pattern);
}

// If we are configured to CF then if we reach depth == DEPTH_START we should reinit the
// currentPattern
if (callDepth == DEPTH_START && _isRule1Activated()) {
currentPattern = PATTERN_START;
if (flowConfig.depth == DEPTH_START && _isRule1Activated()) {
flowConfig.pattern = PATTERN_START;
}

_callDepth = callDepth;
_currentPattern = currentPattern;
_flowConfig = flowConfig;
}

/**
* Check if the current call flow pattern (that is, the result of the rolling hash) is an allowed pattern.
*/
function _checkCallFlow(uint256 pattern) private view {
function _checkCallFlow(uint216 pattern) internal view {
require(_allowedPatterns[pattern], "SphereX error: disallowed tx pattern");
}

Expand Down
2 changes: 1 addition & 1 deletion src/SphereXProtected.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ abstract contract SphereXProtected {
* @param num function identifier
* @return gas used before calling the original function body
*/
function _sphereXValidateInternalPre(int256 num) private returnsIfNotActivated returns(uint256){
function _sphereXValidateInternalPre(int256 num) private returnsIfNotActivated returns (uint256) {
_sphereXEngine().sphereXValidateInternalPre(num);
return gasleft();
}
Expand Down
32 changes: 15 additions & 17 deletions test/SphereXEngine.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,15 +87,15 @@ contract SphereXEngineTest is Test, CFUtils {

function test_addAllowedPatterns_two_patterns() public activateRule(CF) {
int16[2] memory allowed_cf = [int16(1), -1];
uint256 allowed_cf_hash = 1;
uint216 allowed_cf_hash = 1;
for (uint256 i = 0; i < allowed_cf.length; i++) {
allowed_cf_hash = uint256(keccak256(abi.encode(int256(allowed_cf[i]), allowed_cf_hash)));
allowed_cf_hash = uint216(bytes27(keccak256(abi.encode(int256(allowed_cf[i]), allowed_cf_hash))));
}

int16[2] memory allowed_cf_2 = [int16(2), -2];
uint256 allowed_cf_hash_2 = 1;
uint216 allowed_cf_hash_2 = 1;
for (uint256 i = 0; i < allowed_cf_2.length; i++) {
allowed_cf_hash_2 = uint256(keccak256(abi.encode(int256(allowed_cf_2[i]), allowed_cf_hash_2)));
allowed_cf_hash_2 = uint216(bytes27(keccak256(abi.encode(int256(allowed_cf_2[i]), allowed_cf_hash_2))));
}

allowed_patterns = [allowed_cf_hash, allowed_cf_hash_2];
Expand All @@ -116,7 +116,7 @@ contract SphereXEngineTest is Test, CFUtils {

function test_removeAllowedPatterns(bytes8 rule) public activateRule(rule) {
allowed_cf_storage = [int16(1), -1];
uint256 allowed_cf_hash = addAllowedPattern();
uint216 allowed_cf_hash = addAllowedPattern();

allowed_patterns = [allowed_cf_hash];
spherex_engine.removeAllowedPatterns(allowed_patterns);
Expand All @@ -129,11 +129,11 @@ contract SphereXEngineTest is Test, CFUtils {
// remove two cf and check that the first one was removed
function test_removeAllowedPatterns_check_first_pattern_removed() public activateRule(CF) {
allowed_cf_storage = [int16(1), -1];
uint256 allowed_cf_hash = addAllowedPattern();
uint216 allowed_cf_hash = addAllowedPattern();
allowed_cf_storage = [int16(2), -2];
addAllowedPattern();
allowed_cf_storage = [int16(3), -3];
uint256 allowed_cf_hash_3 = addAllowedPattern();
uint216 allowed_cf_hash_3 = addAllowedPattern();

allowed_patterns = [allowed_cf_hash, allowed_cf_hash_3];
spherex_engine.removeAllowedPatterns(allowed_patterns);
Expand All @@ -149,11 +149,11 @@ contract SphereXEngineTest is Test, CFUtils {
// remove two cf and check that the second one was removed
function test_removeAllowedPatterns_check_second_pattern_removed() public activateRule(CF) {
allowed_cf_storage = [int16(1), -1];
uint256 allowed_cf_hash = addAllowedPattern();
uint216 allowed_cf_hash = addAllowedPattern();
allowed_cf_storage = [int16(2), -2];
addAllowedPattern();
allowed_cf_storage = [int16(3), -3];
uint256 allowed_cf_hash_3 = addAllowedPattern();
uint216 allowed_cf_hash_3 = addAllowedPattern();

allowed_patterns = [allowed_cf_hash, allowed_cf_hash_3];
spherex_engine.removeAllowedPatterns(allowed_patterns);
Expand Down Expand Up @@ -334,10 +334,8 @@ contract SphereXEngineTest is Test, CFUtils {
addAllowedPattern();

sendNumberToEngine(1);
assertEq(
vm.load(address(spherex_engine), currentPatternStorageSlot), keccak256(abi.encode(int256(1), uint256(1)))
);
assertEq(vm.load(address(spherex_engine), cfDepthStorageSlot), bytes32(uint256(2)));
assertEq(getCurrentPattern(), uint216(bytes27(keccak256(abi.encode(int256(1), uint256(1))))));
assertEq(getCurrentCallDepth(), uint16(2));

sendNumberToEngine(-1);
assertFlowStorageSlotsInInitialState();
Expand All @@ -354,8 +352,8 @@ contract SphereXEngineTest is Test, CFUtils {
sendNumberToEngine(allowed_cf_storage[i]);
}

assertEq(vm.load(address(spherex_engine), currentPatternStorageSlot) != bytes32(uint256(1)), true);
assertEq(vm.load(address(spherex_engine), cfDepthStorageSlot), bytes32(uint256(1)));
assertEq(getCurrentPattern() != uint216(1), true);
assertEq(getCurrentCallDepth(), uint16(1));
}

// Sanity check, unlike cf the current pattern isnt being cleaned at depth==1 hence this
Expand Down Expand Up @@ -465,8 +463,8 @@ contract SphereXEngineTest is Test, CFUtils {

// the slot layout is 0x[32 empty bits][160 bits for origin address][64 bits for block number]
assertEq(
(vm.load(address(spherex_engine), currentBlockStorageSlot)),
keccak256(abi.encode(2, random_address, block.timestamp, block.difficulty))
getCurrentBlockBoundry(),
bytes3(keccak256(abi.encode(2, random_address, block.timestamp, block.difficulty)))
);
}

Expand Down
4 changes: 2 additions & 2 deletions test/SphereXProtected.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ contract SphereXProtectedTest is Test, CFUtils {
costumer_contract = new CostumerContract();

int16[2] memory allowed_cf = [int16(1), -1];
uint256 allowed_cf_hash = 1;
uint216 allowed_cf_hash = 1;
for (uint256 i = 0; i < allowed_cf.length; i++) {
allowed_cf_hash = uint256(keccak256(abi.encode(int256(allowed_cf[i]), allowed_cf_hash)));
allowed_cf_hash = uint216(bytes27(keccak256(abi.encode(int256(allowed_cf[i]), allowed_cf_hash))));
}
allowed_patterns.push(allowed_cf_hash);
allowed_senders.push(address(costumer_contract));
Expand Down
4 changes: 2 additions & 2 deletions test/SphereXProtectedUpgradeable.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ contract SphereXProtectedProxyTest is Test, SphereXProtectedTest {
costumer_proxy_contract = new CostumerContractProxy(address(costumer_contract));

int16[2] memory allowed_cf = [int16(1), -1];
uint256 allowed_cf_hash = 1;
uint216 allowed_cf_hash = 1;
for (uint256 i = 0; i < allowed_cf.length; i++) {
allowed_cf_hash = uint256(keccak256(abi.encode(int256(allowed_cf[i]), allowed_cf_hash)));
allowed_cf_hash = uint216(bytes27(keccak256(abi.encode(int256(allowed_cf[i]), allowed_cf_hash))));
}
allowed_patterns.push(allowed_cf_hash);
allowed_senders.push(address(costumer_proxy_contract));
Expand Down

0 comments on commit b9e1c2b

Please sign in to comment.