Asymmetric level of transaction validation between L1 side and L2.

Lines of code Vulnerability details Impact May have double counting of deposits? Proof of Concept On L1 side, there are validations that ensure funds don't get lost nor double counted. For example, finalizeWithdrawal may be called multiple times by allowed callers in case previous calls fail...

Add chainId to address creation.

Lines of code Vulnerability details Impact Although it may not be in the plan to deploy multiple zkSync rollups, it makes sense to include chainid in computeCreate2Address from the very start. It would be more EIP-155 compliant, and will prevent future replay attacks, where a tx of first zkSync...

Double spending risk in L1 Bridge Contract

Lines of code Vulnerability details Impact There is double spending risk in L1 Bridge Contract. The user may call claimFailedDeposit to release their locked fund while they still have token balance in L2 network. Proof of Concept Let us focus on the L1ERC20Bridge.sol /// @dev Withdraw funds from...

Upgraded Q -> M from 250 [1667617695647]

Judge has assessed an item in Issue 250 as Medium risk. The relevant finding follows: --- The text was updated successfully, but these errors were encountered: All reactions...

Upgraded Q -> H from 502 [1667614585174]

Judge has assessed an item in Issue 502 as High risk. The relevant finding follows: --- The text was updated successfully, but these errors were encountered: All reactions...

Upgraded Q -> H from 211 [1667662048262]

Judge has assessed an item in Issue 211 as High risk. The relevant finding follows: --- The text was updated successfully, but these errors were encountered: All reactions...

Upgraded Q -> H from 262 [1667614992867]

Judge has assessed an item in Issue 262 as High risk. The relevant finding follows: --- The text was updated successfully, but these errors were encountered: All reactions...

Upgraded Q -> M from 62 [1667612948063]

Judge has assessed an item in Issue 62 as Medium risk. The relevant finding follows: --- The text was updated successfully, but these errors were encountered: All reactions...

Upgraded Q -> M from 407 [1667616784805]

Judge has assessed an item in Issue 407 as Medium risk. The relevant finding follows: --- The text was updated successfully, but these errors were encountered: All reactions...

Upgraded Q -> H from 149 [1667660267515]

Judge has assessed an item in Issue 149 as High risk. The relevant finding follows: --- The text was updated successfully, but these errors were encountered: All reactions...

Upgraded Q -> H from 502 [1667614581063]

Judge has assessed an item in Issue 502 as High risk. The relevant finding follows: --- The text was updated successfully, but these errors were encountered: All reactions...

Upgraded Q -> M from 91 [1667616172726]

Judge has assessed an item in Issue 91 as Medium risk. The relevant finding follows: --- The text was updated successfully, but these errors were encountered: All reactions...

Upgraded Q -> M from 203 [1667614189925]

Judge has assessed an item in Issue 203 as Medium risk. The relevant finding follows: --- The text was updated successfully, but these errors were encountered: All reactions...

Upgraded Q -> M from 207 [1667615741898]

Judge has assessed an item in Issue 207 as Medium risk. The relevant finding follows: --- The text was updated successfully, but these errors were encountered: All reactions...

The governor can use the replay attack to manipuate the votes in his favor

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. Since the argument of function approveEmergencyDiamondCutAsSecurityCouncilMember does not have the proposalID, a governor can use replay attack to manipulate the votes in his favor, that is, even not...

initializer modifier on constructor function

Lines of code Vulnerability details Impact In L2StandardERC20.sol the initializer modifier is placed both on the constructor function and on the bridgeInitialize function. When the initializer modifier is used it cannot be called again on any function in the contract since it will revert with a...

Unchecked array length of the public_inputs[] in Verifier.sol

Lines of code Vulnerability details Impact If an array enumerates all registered addresses, an adversary can register many addresses, causing the problem described above. Vulnerability Details Ethereum is a very resource-constrained environment. Prices per computational step are orders of magnitu...

reentrancyGuardInitializer modifier used on constructor and Initialize functions

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. In L1EthBridge.sol there are two places where the reentrancyGuardInitializer modifier is used. It's found on both the constructor function and also on the initialize function. This is a problem because...

Unchecked array lenght

Lines of code Vulnerability details Impact If an array enumerates all registered addresses, an adversary can register many addresses, causing the problem described above. Description Ethereum is a very resource-constrained environment. Prices per computational step are orders of magnitude higher...

reentrancyGuardInitializer modifier used on constructor and Initialize functions

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. In L1ERC20Bridge.sol there are two places where the reentrancyGuardInitializer modifier is used. It's found on both the constructor function and also on the initialize function. This is a problem becaus...

May introduce an invalid Facet into the system

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. The saveFacetIfNew has not checked if the newly introduced facet is valid or not a valid contract, therefore, an invalid address might be added as a facet. according to Solidity documentation, "If the...

Misleading code in BorrowController including no validation of the amounts to be borrowed.

Lines of code Vulnerability details Impact BorrowController contract checks if any contract is in the borrow list. However, due to the use of msg.sender == tx.origin statement, it returns true for the EOA addresses. It's also observed that any amount of borrowing is acceptable by the...

Compromised or malicious owner of WardenPledge contract can steal pledge creator's deposited reward token amount

Lines of code Vulnerability details Impact There is no guarantee that the owner of the WardenPledge contract does not become compromised or malicious in the future. If this owner becomes compromised or malicious, after a pledge is created and the corresponding reward token amount is deposited, su...

Deprecated ChainLink API

Lines of code Vulnerability details Impact According to Chainlink's documentation, the latestAnswer function is deprecated. This function might suddenly stop working if Chainlink stop supporting deprecated APIs. And the old API can return stale data. Proof of Concept function latestAnswer externa...

Liquidator is paid too much

Lines of code Vulnerability details Impact Liquidator is paid 1 + liquidationIncentive repaidDebt, instead of liquidationIncentive repaidDebt. Proof of Concept liquidatorReward is currently calculated in Market.liquidate as follows: uint liquidatorReward = repaidDebt 1 ether / price;...

No access control for initialize in diamondinit

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. The initialize function has no access control and thus a malicious user can call it an rest all the the parameters of the system. Proof of Concept Provide direct links to all referenced code in GitHub...

Decimals are computed in the wrong way if the collateral token doesn't have 18 decimals

Lines of code Vulnerability details Impact Huge accounting errors and losses for borrowers and liquidators if a collateral token with a non-18 decimal value is used. The oracle contract won't always return prices in 18 decimals. Proof of Concept The oracle returns the price in a normalized way...

DBR can be reused between multiple debtor accounts

Lines of code Vulnerability details Impact A borrower can reuse DBR between various accounts. When a user attempts to call forceReplenish, the borrower will transfer the held DBR to the targeted account. In theory, a borrower can have infinite accounts in DBR debt but enough DBR on a single accou...

If transferring DBR after borrowing DOLA in same transaction, DBR amount that is used already for borrowing DOLA can still be shared to allow users who do not own DBR to borrow DOLA

Lines of code Vulnerability details Impact Although my other finding named "DOLA can be borrowed without owning any DBR" indicates that users who do not own any DBR can still borrow DOLA, the design still appears to only allow the DBR owners to borrow DOLA; this is confirmed by , which states tha...

Can steal Fed contract's DOLA balance

Lines of code Vulnerability details Description Due to lack of protection control, it is possible to steal Fed contract's DOLA balance by using a malicious attackerMarket contract by callingFedtakeProfit public function. function takeProfitIMarket market public uint profit = getProfitmarket;...

The pledge creators might lose all of their funds by recoverERC20().

Lines of code Vulnerability details Impact There is a recoverERC20 function to withdraw ERC20 tokens from the contract. Currently, it checks if the token isn't an active reward token but it can be passed easily if the admin removes the reward token using removeRewardToken. So if the admin removes...

Front-running of a pledge call

Lines of code Vulnerability details Description There is pledge function in the WardenPledge contract. The function accepts the pledgeId, which does not contain any information about the pledge itself. As a result, transactions of users can be front-runned to enforce them use the pledge with the...

Owner can steal all the rewards token from the WardenPledge.sol smart contract and break the internal accounting

Lines of code Vulnerability details Impact The owner of the WardenPledge.sol smart contract can steal all the reward tokens from the contract and break the internal accounting. With the recoverERC20 function, the owner can transfer to him/herself the whole balance of the token. The check at L654...

Pledge creator can extend pledge without paying in edge cases.

Lines of code Vulnerability details Impact Pledge creator can extend pledge without paying in edge cases. Proof of Concept When pledge creators wants to extend their pledges, they must transfer an additional reward amount and fee: uint256 totalRewardAmount = pledgeParams.rewardPerVote...

SimpleERC20Escrow's initialize() can be frontrun

Lines of code Vulnerability details Impact The user's collateral is not held in the market contract but is instead held in individual escrows. Every user has a unique escrow for every market. And the escrow contracts are created via the Market contract's createEscrow function. And it's initialize...

Chainlink latestAnswer is deprecated

Lines of code Vulnerability details Impact Since Chainlink latestAnswer is deprecated, use latestRoundData instead. Since it is deprecated, there is chance of Chainlink to stop supporting this function. Proof of Concept uint price = feedstoken.feed.latestAnswer; Tools Used Manual Analysis...

Protocol's usability becomes very limited when access to Chainlink oracle data feed is blocked

Lines of code Vulnerability details Impact Based on the current implementation, when the protocol wants to use Chainlink oracle data feed for getting a collateral token's price, the fixed price for the token should not be set. When the fixed price is not set for the token, calling the Oracle...

Wrong reward calculation when reward token's decimals are different than 18

Lines of code Vulnerability details Impact When a user creates a pledge, she can specify the maximum amount of the Total Rewards and the maximum amount of fee amount she is willing to spend. By using the rewardPerVote, the vars.votesDifference and the vars.duration the smart contract calculates...

Using a non-18 decimals token as collateral (for ex. USDT) in a Market will result in multiple value losing situations

Lines of code Vulnerability details Proof of Concept In multiple places in the code, when doing calculations with both debt and price of collateral there is a multiplication by 1e18 - 1 ether. We have the following calculations: uint minimumCollateral = debt 1 ether /...

Admin can set very low or very high value for setting fees & collateral factor in Market.sol resulting in value loss and/or DoS

Lines of code Vulnerability details Proof of Concept The contract’s admin has control to set values in setCollateralFactorBps, setLiquidationFactorBps, setReplenismentIncentiveBps, setLiquidationIncentiveBps, setLiquidationFeeBps. All of them have upper bounds, most of them have lower bounds, but...

No access control INVEscrow.onDeposit() --> infinite balance xINV.

Lines of code Vulnerability details Impact No access control on INVEscrow.onDeposityou can call recursively the function to artificially increase your xINV balance. Proof of Concept Any user with a non null balance on its escrow can call the function infinitely to mint xINV tokens via...

1-phase governor changing in Fed

Lines of code Vulnerability details Impact Mistake in calling this function setting 0-address or just wrong address will lead to full control loosing Tools Used vs code Recommended Mitigation Steps Do 2-phase changing like in DolaBorrowingRights.setPendingOperator and...

Owner can withdraw all funds from a WardenPledge contract

Lines of code Vulnerability details Impact recoverERC20 is meant to allow the owner to recover funds sent by mistake to the contract, and contains a check to make sure funds can't be withdrawn for whitelisted rewards tokens. However because the owner can unwhitelist tokens and no checks exist in...

operator role it's handled in a way that can damage the protocol

Lines of code Vulnerability details operator role it's handled in a way that can damage the protocol Impact Same logic is being deployed several times, for maintainability is hardly recommended to not copy paste same logic many times but inherit it. Also there is an issue in BorrowController with...

Repayment/Burn of due tokens is not enforced

Lines of code Vulnerability details Impact Repayment/burning of due DBR tokens is currently not enforced, which enables a user to simply switch addresses to extend their borrowing duration, which eventually leads to almost 0% interest paid if performed repeatedly, breaking one of the core...

Timelock Contract should be used to avoid malicious governance

Lines of code Vulnerability details Impact Governance of Market.sol can call following function at anytime. This is not ideal since they can call this function for their own benefits. For example they can change liquidationFactorBps to gain more liquidationFee. They can change collateralFactorBps...

Owner can drain pledged tokens balance with recoverERC20 function

Lines of code Vulnerability details Impact The recoverERC20 function allows the owner to withdraw the ERC20 tokens sent by acceident to the contract but it doesn't allow him to withdraw pldged tokens, the owner though could use the removeRewardToken function to remove a token used currently in a...

Pausing WardenPledge contract, which takes effect immediately, by its owner can unexpectedly block pledge creator from calling closePledge or retrievePledgeRewards function

Lines of code Vulnerability details Impact The owner of the WardenPledge contract is able to call the pause function to pause this contract. When the WardenPledge contract is paused, calling the closePledge or retrievePledgeRewards function that uses the whenNotPaused modifier reverts, and the...

Oracle's getPrice() returns the wrong price in certain combinations of oracle feed and token decimals

Lines of code Vulnerability details Impact The oracle's getPrice returns a price that is off by several orders of magnitued depending of the combination of decimals for the Chainlink's pricefeed and underlaying token's pricefeed Actually it based on the decimals set when adding the feed tot he...

Liquidatin function does not consider cases where the liquidate user does not have enough collateral to pay.

Lines of code Vulnerability details Impact Liquidatin function does not consider cases where the liquidate user does not have enough collateral to pay. Proof of Concept The problem is the paid collateral in determined by repaiddebt: I ingnored decimals for simplifcation of the formulas I'm gonna...