Arbitrary call order to handle mutual consent can lead to unrecoverable native ETH

Lines of code Vulnerability details Creating new credits and increasing the credit deposit requires both parties, the lender and the borrower, to agree. This is implemented by having both call the same function with the same call data. However, as it's possible to use native ETH as a credit token...

SpigotLib._claimRevenue is marked public instead of internal

Lines of code Vulnerability details Impact SpigotLib.claimRevenue is marked public instead of internal. This public function is wrapped in the external claimRevenue function. Attacker can call claimRevenue to claim Revenue Tokens into the Spigot escrow for later withradrawal. Tools Used Manual...

When borrower repays, it can overflow and make them owe 2^256 tokens to lender.

Lines of code Vulnerability details Description CreditLib's repay function is the actual accounting of repayments in a LineOfCredit: function repay ILineOfCredit.Credit memory credit, bytes32 id, uint256 amount external returns ILineOfCredit.Credit memory unchecked if amount =...

Use of .transfer()

Lines of code Vulnerability details Impact Note: I realize that this was flagged in the C4udit output, though it seems to believe that it's an ERC20 transfer. The use of transfer to send ETH only forwards 2300 gas to the recipient. This is not enough gas to execute a gnosis safe delegatecall. Fun...

Logic in SpigotLib will not work with rebasing tokens

Lines of code Vulnerability details Proof of Concept Some tokens may make arbitrary balance modifications outside of transfers e.g. Ampleforth style rebasing tokens, Compound style airdrops of governance tokens, mintable / burnable tokens. The code in the claimEscrow function uses a previously...

Borrower can mark his line of credit as repaid by calling line.close(fakeId) repeatedly

Lines of code Vulnerability details Description This vulnerability can be summarized in the following 4 observations: 1. There are no checks to prevent calling line.close with id that doesn’t correspond to any open position. 2. The effect of calling line.close with a fake position id is...

## Potential for loops hitting the block gas limit

Lines of code Vulnerability details Proof of Concept A malicious/compromised LineOfCredit arbiter can add a huge amount of collaterals to the Escrow by calling enableCollateral. This method pushes each collateral token to an array that is then iterated over in getCollateralValue. If the array siz...

Missing ReEntrancy Guard to sendOutTokenOrETH function

Lines of code Vulnerability details Proof of Concept There is no re-entry risk on true ERC-20 tokens that work according to the spec i.e. audited, etc.. However you can write a malicious ERC-20 with custom transferFrom or approve that have re-entrancy hooks to attack a target. Furthermore ERC-777...

When lender consents before borrower in ETH credit token, all the lent funds are permanently lost.

Lines of code Vulnerability details Description The addCredit function transfers money from lender to a LineOfCredit contract, and opens a credit account. increaseCredit transfers additional funds to an existing credit account contract. Both functions are payable and guarded by mutualConsent, whi...

Not checking amount of received ERC20 tokens

Lines of code Vulnerability details Description There are ERC20 tokens, which apply intenal fee on transfers such as USDT, so that amount of received tokens could differ from requested amount. There is a line where contract receives facilityFee as an amount of tokens. The same variable is used on...

State varaible marketplace has never been initialized in LooksRareProxy.sol

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. State varaible marketplace has never been initialized in LooksRareProxy.sol. Although it has been initialized in the constructor, it won't count since functions such as execute will only be called via...

Potential DoS when closing a credit nominated in ETH in the LineOfCredit contract

Lines of code Vulnerability details When closing a credit that was issued in ETH, the LineOfCredit contract will send the lender his deposit and any accrued interests using the address.transferamount function, which may fail and revert the whole function, leading to an eventual DoS. Impact The...

call() should be used instead of transfer() on address payable

Lines of code Vulnerability details Proof of Concept The use of the deprecated transfer function for an address will inevitably make the transaction fail when: 1. The claimer smart contract does not implement a payable function. 2. The claimer smart contract does implement a payable fallback whic...

Closing a single credit line can be reentered and allows closing all open credit lines

Lines of code Vulnerability details Closing a single credit line can be reentered and allows decrementing count as many times as needed to reach the value 0. As soon as the value is 0, the status of all credit lines will be updated to LineLib.STATUS.REPAID. Reentrancy is possible by the borrower ...

Using keccak256(abi.encoded()) can result in collisions

Lines of code Vulnerability details Proof of Concept The code in mutualConsent makes use of keccak256abi.encodePacked but this can result in a collision when the arguments of abi.encodePacked are aligned in a way that gives the same result. Impact Having hash collisions in mutualConsent...

[NAZ-M2] Lack of Price Freshness Check In Oracle.sol#latestRoundData() Allows A Stale Price To Be Used

Lines of code Vulnerability details Impact Oracle.sol should use the updatedAt value from the Oracle.sollatestRoundData function to make sure that the latest answer is recent enough to be used. Proof of Concept In the current implementation of Oracle.sollatestRoundData, there is no freshness chec...

No Allowlist For Bridgeable ERC-20 Tokens

Lines of code Vulnerability details Vulnerability Details We noticed that the deposit function of the L1ERC20Bridge contract code snippet 1 permits a user to bridge any ERC-20 tokens including deflationary and rebase tokens from the L1 to the L2 network. We considered that permitting non-standard...

WHEN EVER CALLING bridgeMint OR bridgeBurn FUNCTIONS NEED TO CHECK THE STATUS OF THE TRANSACTIONS

Lines of code Vulnerability details Impact If not checking the status of the bridgeMint and bridgeBurn functions we don't know the the transactions are success or not. If function breaks any of the reasons we don't what happened and what is the status of the transactions. there is no solutions...

Uninitializing Bridge Contracts' State Variables

Lines of code Vulnerability details Vulnerability Details The L1ERC20Bridge and L1EthBridge are implementation contracts that would be delegatecalled by their corresponding proxy contracts. In other words, all state variables and assets would be stored in the proxy contracts. In contrast, the...

No Checks On Withdraw Function For _amount

Lines of code Vulnerability details Impact If bridgeburn function is a faulty function or a contract that uses older version of solidity the variable the balance may overflow due to lack of checks. Proof of Concept In the function there should be a check balanceOfmsg.sender=amount to mitigate...

Mssing Crucial Checks When Unlocking funds for Withdraw Requests from L2

Lines of code Vulnerability details Impact Atomicity literally does not exist when users from L2 initiate a withdrawal by burning funds on the contract and sending the message to L1. This is giving malicious attackers plenty of time to stealthily launch a series of small and yet sizable forgery o...

Validator can revert block at no cost.

Lines of code Vulnerability details Impact Validator can revert block at no cost. Proof of Concept the validator should only call this function on Executor.sol to revert the unexecuted blocks if the block is not really not executable. /// @notice Reverts unexecuted blocks /// @param newLastBlock...

_safeMint should be used to work with smart contract address

Lines of code Vulnerability details Fix safeMint should be used if you are sending the minted token to a Contract that is capable to manage NFTs or not. This is to prevent tokens to be lost. Recommended Mitigation Steps Replace mint with safeMint. Tools Used Manual review --- The text was updated...

L2 deposits might be reverted

Lines of code Vulnerability details Impact L1ERC20Bridge and L1EthBridge contracts have claimFailedDeposit function to withdraw funds from the initiated deposit, which failed when finalizing on L2. However, if the caller is allowed in senderCanCallFunction modifier, this function can be used to...

BLOCK_PERIOD is incorrect

Lines of code Vulnerability details The BLOCKPERIOD is set to 13 seconds in Config.sol. uint256 constant BLOCKPERIOD = 13 seconds; Since moving to Proof-of-Stake PoS after the Merge, block times on ethereum are fixed at 12 seconds per block slots. . Impact This results in incorrect calculation of...

uint16 type for the facet position and selector position

Lines of code Vulnerability details Description In the Diamond library there is uint16 type used for the facet position and selector position in the FacetToSelectors and SelectorToFacet structs. That creates a restriction that the number of facets is limited by 2^16. In case when the number of...

No commitment to data passed as input to the init contract during the execution of the diamond cut proposal

Lines of code Vulnerability details Description There is a function executeDiamondCutProposal in the DiamondCutFacet contract. It checks that proposal data passed as input to this call is equal to the data that is declared when the creation of this proposal using the following logic: require...

ERC20 and ETH might be returned back to the wrong originator

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. Assume the address of erc20EnabledLooksRareAggregator was maliciously changed to fakeerc20EnabledLooksRareAggregator during an execution of LooksRareAggregator.execute then, the malicious user might for...

No sanity check on _l1Token can lead into a bad deployment on the bridge

Lines of code Vulnerability details No checks on l1Token can lead into a bad deployment on the bridge Impact deployL2Token calls bridgeInitialize which is only callable one time for token as it uses initializer modifier. There are no checks for 0 address which can be assigned by error as it is a...

Governor ownership can be lost because of not sanity check

Lines of code Vulnerability details Governor ownership can be lost because of no checks Impact Sanity checks are important to not affect reputation / flows and users of the protocol when a mistake is done. 0 address should be checked for important address assignments in this case, only done in th...

Potential DoS in unbounded for loop

Lines of code Vulnerability details Proof of Concept The function facets in Getters.sol iterates over the DiamondStorage facets array. In Diamond.sol we can see that by calling diamondCut in a way that saveFacetIfNew gets called, the diamond owner can add an indefinite amount of entries. If this ...

L1ERC20Bridge does not have an emergency stop mechanism to pause the bridge in case of any problems found.

Lines of code Vulnerability details Impact Right now, L1ERC20Bridge implements nonReentrant and access control mechanism to improve security. However, there is not any emergency stop mechanism that can be triggered by an authorized account to pause the bridge in case of some problems. It is highl...

Unsafe Initializations Of Bridge Contracts

Lines of code Vulnerability details Vulnerability Details During the zkSync initialization process, several complicated tasks would be required to execute. Incorrect configurations in some tasks could lead to unexpected vulnerabilities. One task of the zkSync initialization process is deploying a...

Not possible to DiamondCut in frozen status

Lines of code Vulnerability details Impact DiamondCut's executeDiamondCutProposal executes the proposed diamondCut calldata in the Diamond is unfrozen condition. However, if there are any possible situations that require restriction implementation in which the upgrade should be executed in a froz...

1) THERE IS CONFLICT IN REQUIRE STATMENT. IN finalizeDeposit FUNCTION CHECKS require (msg.sender == l1Bridge, "mq"). IN bridgeMint FUNCTION IMPLEMENTATIONS onlyBridge Modifier CHECKS require(msg.sender == l2Bridge).

Lines of code Vulnerability details 1. Impact Let assume if we calling bridgeMint from finalizeDeposit function the msg.sender is l1bridge . if the msg.sender is not l1bridge can't run the finalizeDeposit but as per bridgeMint the msg.sender should be l2bridge. So it will cause the conflict . the...

A total of 3 risks: one with imperfect concurrency judgment logic, one with unrobust business logic, and one with unreasonable timestamp settings

Lines of code Vulnerability details RISK 1: When creating an auction, if the exact same token is transferred to the contract in a very short period of time, it will cause one of the auctions to fail to be created. is caused by the following statement: if balanceAfterTransfer - balanceBeforeTransf...

Draining baseToken from contract by calling finalize function multiple times

Lines of code Vulnerability details Impact Draining baseToken from SizeSealed contract by calling finalize function multiple times Proof of Concept The finalize function can be called multiple times by providing clearingQuote to typeuint128.max. Currently inside finalize function there is no chec...

Funds can be stolen

Lines of code Vulnerability details Impact For L2, any deposit finalization should come from L1 bridge address. Now the MailBox contract is currently helping to simulate such calls to be coming from L1. The problem is if within zkSync ecosystem, a contract is deployed with address same as L1 brid...

Seller can steal all the bid fund by repeatedly calling finalize()

Lines of code Vulnerability details Impact Bidders' fund will be stolen by malicious seller, they can not get refund. Proof of Concept In finalize, FinalizeData memory data is local, which means in each call, data is re-initialized. By calling finalize multiple times, a malicious seller can get t...

Unsafe cast blocks withdraw of tokens

Lines of code Vulnerability details Unsafe cast blocks withdraw of tokens Impact tokensAvailableAtTime is a internal function used by tokensAvailableForWithdrawal and this one being used internally also externally as it is public by withdraw method. cliffAmount is a uint256, being able to have mu...

[PNM-002] finalize with malicious input may lock bidder funds in the contract

Lines of code Vulnerability details Description The finalize function of the contract SizeSealed is used to finalize an auction, allowing the auctioner or seller to be paid quote tokens and also eventually allowing successful bidders to withdraw base tokens. Once the finalize function is called,...

Multi Step upgrades introduce security risk

Lines of code Vulnerability details Impact If Governor upgrades the system using executeDiamondCutProposal function then freezing is removed. This could be a problem where Governor was still not ready to make freezable facet available as shown in POC Proof of Concept 1. Governor discovers a...

Bid can be cancelled after being finalized

Lines of code Vulnerability details Impact The SizeSealed contract's cancelBid function can be called after auction has been finalized, thereby allowing the bidder to withdraw their quoteToken after bid is finalized This is possible by bypassing the if a.data.lowestQuote != typeuint128.max ||...

Wrong bidIndex calculation

Lines of code Vulnerability details Impact Return wrong bidIndex Proof of Concept SizeSealed.sol Tools Used Manual Recommended Mitigation Steps uint256 bidIndex = a.bids.length - 1 ; --- The text was updated successfully, but these errors were encountered: 👎 1 trust1995 reacted with thumbs down...

Solmate safetransfer and safetransferfrom doesnot check the codesize of the token address, which may lead to fund loss

Lines of code Vulnerability details Impact In bid function, the safetransferfrom function doesn't check the existence of code at the token address. This is a known issue while using solmate's libraries. Hence this may lead to miscalculation of funds and may lead to loss of funds , because if...

Auction can be DoS-ed with too many bids

Lines of code Vulnerability details Proof of Concept The bid functionality pushes every new bid to the a.bids array. On finalize this array is looped over. Now, if there are too many bids in it, the for loop gas consumption will go over the block gas limit, which is 30M gas units currently. The...

Malicious seller can steal from bidders.

Lines of code Vulnerability details Impact A seller can cancel the auction after finalize and thus can steal money from the bidders and get their original baseToken back. POC When an auction is started the value of a.data.lowestQuote is set as typeuint128.max here . In the atState function this...

Some bidders can withdraw would claim moreof the baseToken when auction CliffPercent value is 1

Lines of code Vulnerability details Impact If an auction is created with a.timings.cliffPercent set to 1 , then during withdrawal of baseTokens after auction ends, the some bidders to withdraw would claim more tokens than others of the baseToken amounts. Proof of Concept 1. Auction A is created...

Seller can stole users assets by create and then cancel the auction

Lines of code Vulnerability details Impact Seller can stole users assets by create and cancel auction Proof of Concept Seller can create an auction, then wait for people to participate in auction bidding, finally the seller cancel the auction and get the users assets. This scenario can happen wit...

Attacker can steal any funds in the contract by state confusion (no preconditions)

Lines of code Vulnerability details HIGH: Attacker can steal any funds in the contract by state confusion no preconditions LOC: Description Auctions in SIZE can be in one of several states, as checked in the atState modifier: modifier atStateAuction storage a, States state if block.timestamp...