Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge the changes related to security updates to the master branch #131

Merged
merged 31 commits into from
Jan 3, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
5bd6f5d
Add opposite side limits WIP
patitonar Nov 23, 2018
42a87e6
Add total executed per day
patitonar Nov 26, 2018
aa2470a
Add fixAssetsAboveLimits method
patitonar Nov 28, 2018
7c07cde
Refactor execution limits names
patitonar Nov 28, 2018
414e437
Fix compile warnings
patitonar Nov 28, 2018
492735b
check status of the contract deployment operation as integer
Dec 1, 2018
6ece5f7
correct formatting for configuration examples
Dec 1, 2018
4acb11a
fixed formatting for .env examples in README
Dec 1, 2018
c797817
Merge pull request #114 from akolotov/deploy-status-as-integer
akolotov Dec 3, 2018
30bfcc1
Merge pull request #115 from akolotov/fixes-in-env-example
akolotov Dec 3, 2018
12d0422
Add check on ErcToNative onFailedAffirmation method
patitonar Dec 3, 2018
b7991b7
Refactor overdraw management
patitonar Dec 3, 2018
6ad631b
Refactor storage of out of limit values
patitonar Dec 4, 2018
ab07c56
Fix on onFailedAffirmation method
patitonar Dec 4, 2018
e4a3cf4
Update fixAssetsAboveLimits modifier to onlyProxyOwner
patitonar Dec 4, 2018
4612b7b
Update contract version
patitonar Dec 4, 2018
6069555
Fix OverdrawManagement inheritance
patitonar Dec 10, 2018
abbde31
Update owner roles on bridge contracts
patitonar Dec 10, 2018
6c7320d
Remove Validatable from OverdrawManagement
patitonar Dec 11, 2018
adb81cf
Update claimTokens methods modifier to onlyProxyOwner
patitonar Dec 11, 2018
ac0e671
Refactor onlyProxyOwner modifier
patitonar Dec 14, 2018
b25c25c
Update onlyProxyOwner modifier to onlyIfOwnerOfProxy
patitonar Dec 14, 2018
8ea0333
Fix incorrect storage reference on setExecutionMaxPerTx
patitonar Dec 17, 2018
8bf7c1a
Add OverdrawManagement to ERC20-TO-ERC20 contracts
patitonar Dec 20, 2018
06a9dcf
Add execution limits to NATIVE-TO-ERC20 mode
patitonar Dec 21, 2018
ba4b62c
Update documentation with new roles
patitonar Dec 21, 2018
34f88f9
Update Native-to-Erc20 deploy script
patitonar Dec 21, 2018
6bf96d3
Update ERC20-To-Native deploy script
patitonar Dec 21, 2018
e5d1522
Update ERC20-TO-ERC20 deploy script
patitonar Dec 21, 2018
b4c1a8e
Fix linter errors on deploy scripts
patitonar Dec 21, 2018
416a35d
Merge pull request #117 from poanetwork/111-security-roles-2.2
akolotov Jan 3, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions contracts/IOwnedUpgradeabilityProxy.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pragma solidity 0.4.24;


interface IOwnedUpgradeabilityProxy {
function proxyOwner() public view returns (address);
}
40 changes: 37 additions & 3 deletions contracts/upgradeable_contracts/BasicBridge.sol
Original file line number Diff line number Diff line change
@@ -1,20 +1,23 @@
pragma solidity 0.4.24;
import "../IBridgeValidators.sol";
import "./OwnedUpgradeability.sol";
import "../upgradeability/EternalStorage.sol";
import "../libraries/SafeMath.sol";
import "./Validatable.sol";
import "./Ownable.sol";
import "openzeppelin-solidity/contracts/token/ERC20/ERC20Basic.sol";


contract BasicBridge is EternalStorage, Validatable {
contract BasicBridge is EternalStorage, Validatable, Ownable, OwnedUpgradeability {
using SafeMath for uint256;

event GasPriceChanged(uint256 gasPrice);
event RequiredBlockConfirmationChanged(uint256 requiredBlockConfirmations);
event DailyLimitChanged(uint256 newLimit);
event ExecutionDailyLimitChanged(uint256 newLimit);

function getBridgeInterfacesVersion() public pure returns(uint64 major, uint64 minor, uint64 patch) {
return (2, 1, 0);
return (2, 2, 0);
}

function setGasPrice(uint256 _gasPrice) public onlyOwner {
Expand Down Expand Up @@ -49,6 +52,14 @@ contract BasicBridge is EternalStorage, Validatable {
return uintStorage[keccak256(abi.encodePacked("totalSpentPerDay", _day))];
}

function setTotalExecutedPerDay(uint256 _day, uint256 _value) internal {
uintStorage[keccak256(abi.encodePacked("totalExecutedPerDay", _day))] = _value;
}

function totalExecutedPerDay(uint256 _day) public view returns(uint256) {
return uintStorage[keccak256(abi.encodePacked("totalExecutedPerDay", _day))];
}

function minPerTx() public view returns(uint256) {
return uintStorage[keccak256(abi.encodePacked("minPerTx"))];
}
Expand All @@ -57,6 +68,10 @@ contract BasicBridge is EternalStorage, Validatable {
return uintStorage[keccak256(abi.encodePacked("maxPerTx"))];
}

function executionMaxPerTx() public view returns(uint256) {
return uintStorage[keccak256(abi.encodePacked("executionMaxPerTx"))];
}

function setInitialize(bool _status) internal {
boolStorage[keccak256(abi.encodePacked("isInitialized"))] = _status;
}
Expand All @@ -78,6 +93,20 @@ contract BasicBridge is EternalStorage, Validatable {
return uintStorage[keccak256(abi.encodePacked("dailyLimit"))];
}

function setExecutionDailyLimit(uint256 _dailyLimit) public onlyOwner {
uintStorage[keccak256(abi.encodePacked("executionDailyLimit"))] = _dailyLimit;
emit ExecutionDailyLimitChanged(_dailyLimit);
}

function executionDailyLimit() public view returns(uint256) {
return uintStorage[keccak256(abi.encodePacked("executionDailyLimit"))];
}

function setExecutionMaxPerTx(uint256 _maxPerTx) external onlyOwner {
require(_maxPerTx < executionDailyLimit());
uintStorage[keccak256(abi.encodePacked("executionMaxPerTx"))] = _maxPerTx;
}

function setMaxPerTx(uint256 _maxPerTx) external onlyOwner {
require(_maxPerTx < dailyLimit());
uintStorage[keccak256(abi.encodePacked("maxPerTx"))] = _maxPerTx;
Expand All @@ -93,7 +122,12 @@ contract BasicBridge is EternalStorage, Validatable {
return dailyLimit() >= nextLimit && _amount <= maxPerTx() && _amount >= minPerTx();
}

function claimTokens(address _token, address _to) public onlyOwner {
function withinExecutionLimit(uint256 _amount) public view returns(bool) {
uint256 nextLimit = totalExecutedPerDay(getCurrentDay()).add(_amount);
return executionDailyLimit() >= nextLimit && _amount <= executionMaxPerTx();
}

function claimTokens(address _token, address _to) public onlyIfOwnerOfProxy {
require(_to != address(0));
if (_token == address(0)) {
_to.transfer(address(this).balance);
Expand Down
22 changes: 14 additions & 8 deletions contracts/upgradeable_contracts/BasicForeignBridge.sol
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,18 @@ contract BasicForeignBridge is EternalStorage, Validatable {
bytes32 txHash;
address contractAddress;
(recipient, amount, txHash, contractAddress) = Message.parseMessage(message);
require(contractAddress == address(this));
require(!relayedMessages(txHash));
setRelayedMessages(txHash, true);
require(onExecuteMessage(recipient, amount));
emit RelayedMessage(recipient, amount, txHash);
if (messageWithinLimits(amount)) {
require(contractAddress == address(this));
require(!relayedMessages(txHash));
setRelayedMessages(txHash, true);
require(onExecuteMessage(recipient, amount));
emit RelayedMessage(recipient, amount, txHash);
} else {
onFailedMessage(recipient, amount, txHash);
}
}

function onExecuteMessage(address, uint256) internal returns(bool){
// has to be defined
}
function onExecuteMessage(address, uint256) internal returns(bool);

function setRelayedMessages(bytes32 _txHash, bool _status) internal {
boolStorage[keccak256(abi.encodePacked("relayedMessages", _txHash))] = _status;
Expand All @@ -35,4 +37,8 @@ contract BasicForeignBridge is EternalStorage, Validatable {
function relayedMessages(bytes32 _txHash) public view returns(bool) {
return boolStorage[keccak256(abi.encodePacked("relayedMessages", _txHash))];
}

function messageWithinLimits(uint256) internal view returns(bool);

function onFailedMessage(address, uint256, bytes32) internal;
}
53 changes: 32 additions & 21 deletions contracts/upgradeable_contracts/BasicHomeBridge.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,27 +16,31 @@ contract BasicHomeBridge is EternalStorage, Validatable {
event CollectedSignatures(address authorityResponsibleForRelay, bytes32 messageHash, uint256 NumberOfCollectedSignatures);

function executeAffirmation(address recipient, uint256 value, bytes32 transactionHash) external onlyValidator {
bytes32 hashMsg = keccak256(abi.encodePacked(recipient, value, transactionHash));
bytes32 hashSender = keccak256(abi.encodePacked(msg.sender, hashMsg));
// Duplicated affirmations
require(!affirmationsSigned(hashSender));
setAffirmationsSigned(hashSender, true);

uint256 signed = numAffirmationsSigned(hashMsg);
require(!isAlreadyProcessed(signed));
// the check above assumes that the case when the value could be overflew will not happen in the addition operation below
signed = signed + 1;

setNumAffirmationsSigned(hashMsg, signed);

emit SignedForAffirmation(msg.sender, transactionHash);

if (signed >= requiredSignatures()) {
// If the bridge contract does not own enough tokens to transfer
// it will couse funds lock on the home side of the bridge
setNumAffirmationsSigned(hashMsg, markAsProcessed(signed));
require(onExecuteAffirmation(recipient, value));
emit AffirmationCompleted(recipient, value, transactionHash);
if (affirmationWithinLimits(value)) {
bytes32 hashMsg = keccak256(abi.encodePacked(recipient, value, transactionHash));
bytes32 hashSender = keccak256(abi.encodePacked(msg.sender, hashMsg));
// Duplicated affirmations
require(!affirmationsSigned(hashSender));
setAffirmationsSigned(hashSender, true);

uint256 signed = numAffirmationsSigned(hashMsg);
require(!isAlreadyProcessed(signed));
// the check above assumes that the case when the value could be overflew will not happen in the addition operation below
signed = signed + 1;

setNumAffirmationsSigned(hashMsg, signed);

emit SignedForAffirmation(msg.sender, transactionHash);

if (signed >= requiredSignatures()) {
// If the bridge contract does not own enough tokens to transfer
// it will couse funds lock on the home side of the bridge
setNumAffirmationsSigned(hashMsg, markAsProcessed(signed));
require(onExecuteAffirmation(recipient, value));
emit AffirmationCompleted(recipient, value, transactionHash);
}
} else {
onFailedAffirmation(recipient, value, transactionHash);
}
}

Expand Down Expand Up @@ -144,4 +148,11 @@ contract BasicHomeBridge is EternalStorage, Validatable {
function requiredMessageLength() public pure returns(uint256) {
return Message.requiredMessageLength();
}

function affirmationWithinLimits(uint256) internal view returns(bool) {
return true;
}

function onFailedAffirmation(address, uint256, bytes32) internal {
}
}
51 changes: 51 additions & 0 deletions contracts/upgradeable_contracts/OverdrawManagement.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
pragma solidity 0.4.24;

import "../upgradeability/EternalStorage.sol";
import "../libraries/SafeMath.sol";
import "./OwnedUpgradeability.sol";


contract OverdrawManagement is EternalStorage, OwnedUpgradeability {
using SafeMath for uint256;

event UserRequestForSignature(address recipient, uint256 value);

function fixAssetsAboveLimits(bytes32 txHash, bool unlockOnForeign) external onlyIfOwnerOfProxy {
require(!fixedAssets(txHash));
address recipient;
uint256 value;
(recipient, value) = txAboveLimits(txHash);
require(recipient != address(0) && value > 0);
setOutOfLimitAmount(outOfLimitAmount().sub(value));
if (unlockOnForeign) {
emit UserRequestForSignature(recipient, value);
}
setFixedAssets(txHash, true);
}

function outOfLimitAmount() public view returns(uint256) {
return uintStorage[keccak256(abi.encodePacked("outOfLimitAmount"))];
}

function fixedAssets(bytes32 _txHash) public view returns(bool) {
return boolStorage[keccak256(abi.encodePacked("fixedAssets", _txHash))];
}

function setOutOfLimitAmount(uint256 _value) internal {
uintStorage[keccak256(abi.encodePacked("outOfLimitAmount"))] = _value;
}

function txAboveLimits(bytes32 _txHash) internal view returns(address recipient, uint256 value) {
recipient = addressStorage[keccak256(abi.encodePacked("txOutOfLimitRecipient", _txHash))];
value = uintStorage[keccak256(abi.encodePacked("txOutOfLimitValue", _txHash))];
}

function setTxAboveLimits(address _recipient, uint256 _value, bytes32 _txHash) internal {
addressStorage[keccak256(abi.encodePacked("txOutOfLimitRecipient", _txHash))] = _recipient;
uintStorage[keccak256(abi.encodePacked("txOutOfLimitValue", _txHash))] = _value;
}

function setFixedAssets(bytes32 _txHash, bool _status) internal {
boolStorage[keccak256(abi.encodePacked("fixedAssets", _txHash))] = _status;
}
}
17 changes: 17 additions & 0 deletions contracts/upgradeable_contracts/OwnedUpgradeability.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
pragma solidity 0.4.24;

import "../IOwnedUpgradeabilityProxy.sol";


contract OwnedUpgradeability {

function upgradeabilityAdmin() public view returns (address) {
return IOwnedUpgradeabilityProxy(this).proxyOwner();
}

// Avoid using onlyProxyOwner name to prevent issues with implementation from proxy contract
modifier onlyIfOwnerOfProxy() {
require(msg.sender == upgradeabilityAdmin());
_;
}
}
5 changes: 0 additions & 5 deletions contracts/upgradeable_contracts/Validatable.sol
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,6 @@ contract Validatable is EternalStorage {
_;
}

modifier onlyOwner() {
require(validatorContract().owner() == msg.sender);
_;
}

function requiredSignatures() public view returns(uint256) {
return validatorContract().requiredSignatures();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,27 @@ contract ForeignBridgeErcToErc is BasicBridge, BasicForeignBridge {
address _validatorContract,
address _erc20token,
uint256 _requiredBlockConfirmations,
uint256 _gasPrice
uint256 _gasPrice,
uint256 _maxPerTx,
uint256 _homeDailyLimit,
uint256 _homeMaxPerTx,
address _owner
) public returns(bool) {
require(!isInitialized());
require(_validatorContract != address(0) && isContract(_validatorContract));
require(_requiredBlockConfirmations != 0);
require(_gasPrice > 0);
require(_homeMaxPerTx < _homeDailyLimit);
require(_owner != address(0));
addressStorage[keccak256(abi.encodePacked("validatorContract"))] = _validatorContract;
setErc20token(_erc20token);
uintStorage[keccak256(abi.encodePacked("deployedAtBlock"))] = block.number;
uintStorage[keccak256(abi.encodePacked("requiredBlockConfirmations"))] = _requiredBlockConfirmations;
uintStorage[keccak256(abi.encodePacked("gasPrice"))] = _gasPrice;
uintStorage[keccak256(abi.encodePacked("maxPerTx"))] = _maxPerTx;
uintStorage[keccak256(abi.encodePacked("executionDailyLimit"))] = _homeDailyLimit;
uintStorage[keccak256(abi.encodePacked("executionMaxPerTx"))] = _homeMaxPerTx;
setOwner(_owner);
setInitialize(true);
return isInitialized();
}
Expand All @@ -35,7 +45,7 @@ contract ForeignBridgeErcToErc is BasicBridge, BasicForeignBridge {
return bytes4(keccak256(abi.encodePacked("erc-to-erc-core")));
}

function claimTokens(address _token, address _to) public onlyOwner {
function claimTokens(address _token, address _to) public onlyIfOwnerOfProxy {
require(_token != address(erc20token()));
super.claimTokens(_token, _to);
}
Expand All @@ -45,11 +55,20 @@ contract ForeignBridgeErcToErc is BasicBridge, BasicForeignBridge {
}

function onExecuteMessage(address _recipient, uint256 _amount) internal returns(bool){
setTotalExecutedPerDay(getCurrentDay(), totalExecutedPerDay(getCurrentDay()).add(_amount));
return erc20token().transfer(_recipient, _amount);
}

function setErc20token(address _token) private {
require(_token != address(0) && isContract(_token));
addressStorage[keccak256(abi.encodePacked("erc20token"))] = _token;
}

function messageWithinLimits(uint256 _amount) internal view returns(bool) {
return withinExecutionLimit(_amount);
}

function onFailedMessage(address, uint256, bytes32) internal {
revert();
}
}
Loading