Skip to content

Patch vulnerability from DAO Hacking #73

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.

Sign up for GitHub

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

Open
michaelnguyen102 wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Patch vulnerability from DAO Hacking #73

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
508aa98
Fixed vulnerability from DAO Hacking
michaelnguyen102 Jun 3, 2023
ed13c83
Renamed bridge contract
michaelnguyen102 Jun 3, 2023
2d67eb2
Added script to test vulnerability
michaelnguyen102 Jun 3, 2023
1dd9a32
Added takeFee func
michaelnguyen102 Jun 4, 2023
2766037
Updated takeFee logic for DAO
michaelnguyen102 Jun 4, 2023
126858c
Added validation
michaelnguyen102 Jun 4, 2023
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
202 changes: 97 additions & 105 deletions bridge/contracts/HecBridgeSplitter.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,69 +5,13 @@ import '@openzeppelin/contracts-upgradeable/security/PausableUpgradeable.sol';
import '@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol';
import '@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol';
import '@openzeppelin/contracts-upgradeable/utils/math/SafeMathUpgradeable.sol';
import {OwnableUpgradeable} from '@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol';

interface IOwnableUpgradeable {
function owner() external view returns (address);

function renounceManagement() external;

function pushManagement(address newOwner_) external;

function pullManagement() external;
}

abstract contract OwnableUpgradeable is IOwnableUpgradeable, Initializable, ContextUpgradeable {
address internal _owner;
address internal _newOwner;

event OwnershipPushed(address indexed previousOwner, address indexed newOwner);
event OwnershipPulled(address indexed previousOwner, address indexed newOwner);

/**
* @dev Initializes the contract setting the deployer as the initial owner.
*/
function __Ownable_init() internal onlyInitializing {
__Ownable_init_unchained();
}

function __Ownable_init_unchained() internal onlyInitializing {
_owner = msg.sender;
emit OwnershipPushed(address(0), _owner);
}

function owner() public view override returns (address) {
return _owner;
}

modifier onlyOwner() {
require(_owner == msg.sender, 'Ownable: caller is not the owner');
_;
}

function renounceManagement() public virtual override onlyOwner {
emit OwnershipPushed(_owner, address(0));
_owner = address(0);
}

function pushManagement(address newOwner_) public virtual override {
require(newOwner_ != address(0), 'Ownable: new owner is the zero address');
emit OwnershipPushed(_owner, newOwner_);
_newOwner = newOwner_;
}

function pullManagement() public virtual override {
require(msg.sender == _newOwner, 'Ownable: must be new owner to pull');
emit OwnershipPulled(_owner, _newOwner);
_owner = _newOwner;
}

/**
* @dev This empty reserved space is put in place to allow future versions to add new
* variables without shifting down storage in the inheritance chain.
* See https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps
*/
uint256[49] private __gap;
}
error INVALID_PARAM();
error INVALID_ADDRESS();
error INVALID_AMOUNT();
error INVALID_ALLOWANCE();
error BRIDGE_FAILED();

/**
* @title HecBridgeSplitter
Expand All @@ -76,13 +20,16 @@ contract HecBridgeSplitter is OwnableUpgradeable, PausableUpgradeable {
using SafeMathUpgradeable for uint256;
using SafeERC20Upgradeable for IERC20Upgradeable;

address public LiFiBridge;
address public BridgeContract;
uint256 public CountDest; // Count of the destination wallets
address public DAO;

// Struct Asset Info
struct SendingAssetInfo {
address sendingAssetId;
uint256 sendingAmount;
uint256 totalAmount;
uint feePercentage;
}

/* ======== INITIALIZATION ======== */
Expand All @@ -96,9 +43,10 @@ contract HecBridgeSplitter is OwnableUpgradeable, PausableUpgradeable {
* @dev sets initials
*/
function initialize(uint256 _CountDest, address _bridge) external initializer {
LiFiBridge = _bridge;
if (_bridge == address(0)) revert INVALID_ADDRESS();
if (_CountDest == 0) revert INVALID_PARAM();
BridgeContract = _bridge;
CountDest = _CountDest;
__Ownable_init();
__Pausable_init();
}

Expand All @@ -108,68 +56,112 @@ contract HecBridgeSplitter is OwnableUpgradeable, PausableUpgradeable {

/// @notice Performs a swap before bridging via HECTOR Bridge Splitter
/// @param sendingAssetInfos Array Data used purely for sending assets
/// @param fees Amounts of native coin amounts for bridge
/// @param callDatas CallDatas from lifi sdk
/// @param useSquid use Squid or Lifi
/// @param squidTargetAddress use in executing squid bridge contract
/// @param bridgeFees Amounts of native coin amounts for bridge
/// @param DAOFee Amounts of fee for DAO
/// @param callDatas CallDatas from bridge sdk
function Bridge(
SendingAssetInfo[] memory sendingAssetInfos,
uint256[] memory fees,
bytes[] memory callDatas,
bool useSquid,
address squidTargetAddress
uint256[] memory bridgeFees,
uint256 DAOFee,
bytes[] memory callDatas
) external payable {
require(
sendingAssetInfos.length > 0 &&
if (
sendingAssetInfos.length > 0 &&
sendingAssetInfos.length <= CountDest &&
sendingAssetInfos.length == callDatas.length,
'Splitter: bridge or swap call data is invalid'
);
require(useSquid && squidTargetAddress != address(0), 'Splitter: squid router is invalid');
sendingAssetInfos.length == callDatas.length &&
sendingAssetInfos.length == bridgeFees.length
)
revert INVALID_PARAM();

address callTargetAddress = useSquid ? squidTargetAddress : LiFiBridge;
for (uint256 i = 0; i < sendingAssetInfos.length; i++) {
if (sendingAssetInfos[i].sendingAssetId != address(0)) {
IERC20Upgradeable srcToken = IERC20Upgradeable(sendingAssetInfos[i].sendingAssetId);
bool isFeeEnabled = msg.value > 0 && bridgeFees.length > 0;

require(
srcToken.allowance(msg.sender, address(this)) > 0,
'ERC20: transfer amount exceeds allowance'
);
IERC20Upgradeable srcToken = IERC20Upgradeable(sendingAssetInfos[0].sendingAssetId);
if (srcToken.allowance(msg.sender, address(this)) == 0) revert INVALID_ALLOWANCE();
uint256 totalAmount = _getTotalAmount(sendingAssetInfos) + DAOFee;

srcToken.safeTransferFrom(msg.sender, address(this), sendingAssetInfos[i].sendingAmount);
srcToken.approve(callTargetAddress, sendingAssetInfos[i].sendingAmount);
}
if (srcToken.balanceOf(msg.sender) < totalAmount) revert INVALID_AMOUNT();

srcToken.safeTransferFrom(msg.sender, address(this), totalAmount);
srcToken.approve(BridgeContract, totalAmount);

// Send fee to DAO
if (DAOFee > 0) {
srcToken.safeTransfer(DAO, DAOFee);
}

uint256 length = sendingAssetInfos.length;
for (uint256 i = 0; i < length; i++) {
address sendingAssetId = sendingAssetInfos[i].sendingAssetId;

if (sendingAssetId == address(0)) revert INVALID_ADDRESS();
bytes memory callData = callDatas[i];

if (msg.value > 0 && fees.length > 0 && fees[i] > 0) {
(bool success, ) = payable(callTargetAddress).call{value: fees[i]}(callDatas[i]);
require(success, 'Splitter: bridge swap transaction was failed');
emit CallData(success, callDatas[i]);
uint256 bridgeFee = bridgeFees[i];

//Bridge the rest of the asset to the destination
if (isFeeEnabled && bridgeFee > 0) {
(bool success, ) = payable(BridgeContract).call{value: bridgeFee}(callData);
if (!success) revert BRIDGE_FAILED();
emit MakeCallData(success, callData, msg.sender);
} else {
(bool success, ) = payable(callTargetAddress).call(callDatas[i]);
require(success, 'Splitter: bridge swap transaction was failed');
emit CallData(success, callDatas[i]);
(bool success, ) = payable(BridgeContract).call(callData);
if (!success) revert BRIDGE_FAILED();
emit MakeCallData(success, callData, msg.sender);
}

}

emit HectorBridge(msg.sender, sendingAssetInfos);
}

// Custom counts of detinations
function setCountDest(uint256 _countDest) external onlyOwner {
if (_countDest == 0) revert INVALID_PARAM();
uint256 oldCountDest = CountDest;
CountDest = _countDest;
emit SetCountDest(_countDest);
emit SetCountDest(oldCountDest, _countDest, msg.sender);
}

// Set LiFiDiamond Address
function setBridge(address _bridge) external onlyOwner {
LiFiBridge = _bridge;
emit SetBridge(_bridge);
if (_bridge == address(0)) revert INVALID_ADDRESS();
//check if _bridge is a contract not wallet
uint256 size;
assembly {
size := extcodesize(_bridge)
}
if (size == 0) revert INVALID_ADDRESS();

address oldBridge = BridgeContract;

BridgeContract = _bridge;
emit SetBridge(oldBridge, _bridge, msg.sender);
}

function setDAO(address newDAO) external onlyOwner {
if (newDAO == address(0)) revert INVALID_ADDRESS();
address oldDAO = DAO;

DAO = newDAO;
emit SetDAO(oldDAO, newDAO, msg.sender);
}

///////////////////////////////////////////////////////
// INTERNAL FUNCTIONS //
///////////////////////////////////////////////////////

function _getTotalAmount(SendingAssetInfo[] memory sendingAssetInfos) internal pure returns (uint256) {
uint256 totalAmount = 0;
for (uint256 i = 0; i < sendingAssetInfos.length; i++) {
totalAmount = totalAmount.add(sendingAssetInfos[i].sendingAmount);
}
return totalAmount;
}

// All events
event SetCountDest(uint256 countDest);
event SetBridge(address bridge);
event CallData(bool success, bytes callData);
event HectorBridge(address user, SendingAssetInfo[] sendingAssetInfos);
event SetCountDest(uint256 oldCountDest, uint256 newCountDest, address indexed user);
event SetBridge(address oldBridge, address newBridge, address indexed user);
event SetDAO(address oldDAO, address newDAO, address indexed user);
event MakeCallData(bool success, bytes callData, address indexed user);
event HectorBridge(address indexed user, SendingAssetInfo[] sendingAssetInfos);
event SendFeeToDAO(uint256 feeAmount , SendingAssetInfo[] sendingAssetInfos);
}
71 changes: 44 additions & 27 deletions bridge/scripts/deployHecBridgeSplitter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,33 +1,50 @@
import { BigNumber } from 'ethers';
const hre = require("hardhat");
import { HardhatRuntimeEnvironment } from 'hardhat/types';
import { DeployFunction } from 'hardhat-deploy/types';
import { waitSeconds } from '../helper/helpers';
import { ethers } from 'hardhat';
import { constants } from 'ethers';

async function main() {
const [deployer] = await hre.ethers.getSigners();
const _countDest = 2; // Count of the destination wallets, default: 2
const _bridge = "0x1231DEB6f5749EF6cE6943a275A1D3E7486F4EaE";

console.log("Deploying contracts with the account:", deployer.address);
console.log("Account balance:", (await deployer.getBalance()).toString());
async function getImplementationAddress(proxyAddress: string) {
const implHex = await ethers.provider.getStorageAt(proxyAddress, '0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc');
return ethers.utils.hexStripZeros(implHex);
}

const gas = await ethers.provider.getGasPrice();
console.log("Gas Price: ", gas);
const deployHecBridgeSplitter: DeployFunction = async (hre: HardhatRuntimeEnvironment) => {
const { deployments, ethers } = hre;
const { deploy } = deployments;
const [deployer] = await ethers.getSigners();

const hecBridgeSplitterFactory = await ethers.getContractFactory("HecBridgeSplitter");
console.log("Deploying HecBridgeSplitter Contract...");
const squidRouter = '0xce16f69375520ab01377ce7b88f5ba8c48f8d666';
const CountDest = 2;

const hecBridgeSplitterContract = await hre.upgrades.deployProxy(
hecBridgeSplitterFactory,
[_countDest, _bridge],
{
gas: gas,
initializer: "initialize",
}
);
console.log("HecBridgeSplitter contract deployed to:", hecBridgeSplitterContract.address);
const HecBridgeSplitter = await deploy('HecBridgeSplitter', {
from: deployer.address,
args: [],
log: true,
proxy: {
proxyContract: 'OpenZeppelinTransparentProxy',
execute: {
methodName: 'initialize',
args: [CountDest, squidRouter],
},
},
});
const HecBridgeSplitterImplementation = await getImplementationAddress(HecBridgeSplitter.address);

}
if (hre.network.name !== 'localhost' && hre.network.name !== 'hardhat') {
await waitSeconds(10);
console.log('=====> Verifing ....');
try {
await hre.run('verify:verify', {
address: HecBridgeSplitterImplementation,
contract: 'contracts/HecBridgeSplitter.sol:HecBridgeSplitter',
constructorArguments: [],
});
} catch (_) {}
await waitSeconds(10);
}
};

main().catch((error) => {
console.error(error);
process.exitCode = 1;
});
export default deployHecBridgeSplitter;
deployHecBridgeSplitter.tags = ['HecBridgeSplitter'];
deployHecBridgeSplitter.dependencies = [];
Loading