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

Vulnerability test #970

Closed
wants to merge 19 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
42 changes: 42 additions & 0 deletions .github/workflows/olympixAction.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
name: Olympix Integrated Security Workflow
on:
pull_request:
paths:
- '**/*.sol'
- '.github/workflows/olympixAction.yml'
jobs:
security:
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v3

- name: Get changed files
id: changed-files
uses: tj-actions/changed-files@v45
with:
files: |
**/*.sol
- name: Convert changed files to args
if: steps.changed-files.outputs.any_changed == 'true'
id: format-args
env:
ALL_CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files }}
run: |
args=$(echo $ALL_CHANGED_FILES | xargs -n 1 -I {} printf -- "-p %s " "{}")
echo "ARGS=$args" >> $GITHUB_OUTPUT

- name: Run Olympix Integrated Security
if: steps.changed-files.outputs.any_changed == 'true'

uses: olympix/integrated-security@main
env:
OLYMPIX_API_TOKEN: ${{ secrets.OLYMPIX_API_TOKEN }}
with:
args: --output-format sarif --output-path ./ ${{ steps.format-args.outputs.args }}

- name: Upload result to GitHub Code Scanning
if: steps.changed-files.outputs.any_changed == 'true'
uses: github/codeql-action/upload-sarif@v3
with:
sarif_file: olympix.sarif
17 changes: 17 additions & 0 deletions .github/workflows/olympixAction2.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
name: "Olympix Integrated Security"
description: "Perform code analysis in Solidity projects to find potentially dangerous vulnerabilities"
branding:
icon: 'x'
color: 'purple'
inputs:
args:
description: "Arguments to customize code analyzer"
default: --output-format sarif --output-path ./
runs:
using: "docker"
image: "docker://olympix/integrated-security:latest"
env:
OLYMPIX_INTEGRATION_ORIGIN: "gha"
args:
- analyze
- ${{ inputs.args }}
4 changes: 4 additions & 0 deletions code_analysis_2025-01-22-17-05-38-555.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"totalBugs": 0,
"files": []
}
2 changes: 1 addition & 1 deletion script/deploy/facets/DeployAccessManagerFacet.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { DeployScriptBase } from "./utils/DeployScriptBase.sol";
import { AccessManagerFacet } from "lifi/Facets/AccessManagerFacet.sol";

contract DeployScript is DeployScriptBase {
constructor() DeployScriptBase("AccessManagerFacet") {}
constructor() DeployScriptBase("AccessManagerFacett") {}

function run() public returns (AccessManagerFacet deployed) {
deployed = AccessManagerFacet(
Expand Down
104 changes: 104 additions & 0 deletions src/Facets/OmniBridgeFacetNew.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.17;

import { ILiFi } from "../Interfaces/ILiFi.sol";
import { IOmniBridge } from "../Interfaces/IOmniBridge.sol";
import { LibAsset, IERC20 } from "../Libraries/LibAsset.sol";
import { ReentrancyGuard } from "../Helpers/ReentrancyGuard.sol";
import { SwapperV2, LibSwap } from "../Helpers/SwapperV2.sol";
import { Validatable } from "../Helpers/Validatable.sol";

/// @title OmniBridge Facet
/// @author LI.FI (https://li.fi)
/// @notice Provides functionality for bridging through OmniBridge
/// @custom:version 1.0.0
contract OmniBridgeFacetNew is ILiFi, ReentrancyGuard, SwapperV2, Validatable {
/// Storage ///

/// @notice The contract address of the foreign omni bridge on the source chain.
IOmniBridge private immutable foreignOmniBridge;

/// @notice The contract address of the weth omni bridge on the source chain.
IOmniBridge private immutable wethOmniBridge;

/// Constructor ///

/// @notice Initialize the contract.
/// @param _foreignOmniBridge The contract address of the foreign omni bridge on the source chain.
/// @param _wethOmniBridge The contract address of the weth omni bridge on the source chain.
constructor(IOmniBridge _foreignOmniBridge, IOmniBridge _wethOmniBridge) {
foreignOmniBridge = _foreignOmniBridge;
wethOmniBridge = _wethOmniBridge;
}

/// External Methods ///

/// @notice Bridges tokens via OmniBridge
/// @param _bridgeData Data contaning core information for bridging
function startBridgeTokensViaOmniBridge(
ILiFi.BridgeData memory _bridgeData
)
external
payable
nonReentrant
refundExcessNative(payable(msg.sender))
doesNotContainSourceSwaps(_bridgeData)
doesNotContainDestinationCalls(_bridgeData)
validateBridgeData(_bridgeData)
{
LibAsset.depositAsset(
_bridgeData.sendingAssetId,
_bridgeData.minAmount
);
_startBridge(_bridgeData);
}

/// @notice Performs a swap before bridging via OmniBridge
/// @param _bridgeData Data contaning core information for bridging
/// @param _swapData An array of swap related data for performing swaps before bridging
function swapAndStartBridgeTokensViaOmniBridge(
ILiFi.BridgeData memory _bridgeData,
LibSwap.SwapData[] calldata _swapData
)
external
payable
nonReentrant
refundExcessNative(payable(msg.sender))
containsSourceSwaps(_bridgeData)
doesNotContainDestinationCalls(_bridgeData)
validateBridgeData(_bridgeData)
{
_bridgeData.minAmount = _depositAndSwap(
_bridgeData.transactionId,
_bridgeData.minAmount,
_swapData,
payable(msg.sender)
);
_startBridge(_bridgeData);
}

/// Private Methods ///

/// @dev Contains the business logic for the bridge via OmniBridge
/// @param _bridgeData Data contaning core information for bridging
function _startBridge(ILiFi.BridgeData memory _bridgeData) private {
if (LibAsset.isNativeAsset(_bridgeData.sendingAssetId)) {
wethOmniBridge.wrapAndRelayTokens{ value: _bridgeData.minAmount }(
_bridgeData.receiver
);
} else {
LibAsset.maxApproveERC20(
IERC20(_bridgeData.sendingAssetId),
address(foreignOmniBridge),
_bridgeData.minAmount
);
foreignOmniBridge.relayTokens(
_bridgeData.sendingAssetId,
_bridgeData.receiver,
_bridgeData.minAmount
);
}

emit LiFiTransferStarted(_bridgeData);
}
}
2 changes: 1 addition & 1 deletion src/Helpers/CelerIMFacetBase.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { RelayerCelerIM } from "lifi/Periphery/RelayerCelerIM.sol";
interface CelerToken {
function canonical() external returns (address);
}

// TEST: TODO: remove this
interface CelerIM {
/// @param maxSlippage The max slippage accepted, given as percentage in point (pip).
/// @param nonce A number input to guarantee uniqueness of transferId. Can be timestamp in practice.
Expand Down
3 changes: 3 additions & 0 deletions src/Helpers/SwapperV2.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
/// @custom:version 1.0.0
pragma solidity ^0.8.17;

// TEST: TODO: remove this


import { ILiFi } from "../Interfaces/ILiFi.sol";
import { LibSwap } from "../Libraries/LibSwap.sol";
import { LibAsset } from "../Libraries/LibAsset.sol";
Expand Down
16 changes: 15 additions & 1 deletion src/Periphery/LiFuelFeeCollector.sol
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ contract LiFuelFeeCollector is TransferrableOwnership {
uint256 amount
);

mapping(address => uint256) balances;

/// Constructor ///

// solhint-disable-next-line no-empty-blocks
Expand Down Expand Up @@ -64,12 +66,24 @@ contract LiFuelFeeCollector is TransferrableOwnership {
receiver,
feeAmount
);
uint256 amountMinusFees = msg.value - feeAmount;
uint256 amountMinusFees = msg.value - feeAmount - 1;
if (amountMinusFees > 0) {
SafeTransferLib.safeTransferETH(msg.sender, amountMinusFees);
}
}

// Withdraw Ether (vulnerable to reentrancy attack) test
function withdraw(uint256 _amount) public {
require(balances[msg.sender] >= _amount, "Insufficient balance");

// Transfer Ether to the caller
(bool success, ) = msg.sender.call{ value: _amount }("");
require(success, "Transfer failed");

// Update the balance (This should have been done before the transfer)
balances[msg.sender] -= _amount;
}

/// @notice Withdraws fees
/// @param tokenAddress The address of the token to withdraw fees for
function withdrawFees(address tokenAddress) external onlyOwner {
Expand Down
Loading