Upgraded Q -> 2 from #112 [1686922871117]

Judge has assessed an item in Issue 112 as 2 risk. The relevant finding follows: | | Issue | Instances ---|---|--- M-1 | Centralization Risk for trusted owners | 19 M-1 Centralization Risk for trusted owners Impact: Contracts have owners with privileged rights to perform admin tasks and need to b...

Upgraded Q -> 2 from #16 [1686928129422]

Judge has assessed an item in Issue 16 as 2 risk. The relevant finding follows: L-3 onlyOwner single point of failure Impact The onlyOwner role has a single point of failure and onlyOwner can use critical a few functions. Even if protocol admins/developers are not malicious there is still a chanc...

M-05 Unmitigated

Lines of code Vulnerability details The mitigation makes accrueDrip is disable until the totalSupply 0. But the lastReport blocknumber is not updated. So all the dripped rewards still are collected by the first staker when the drip modifier is called at the second time. Impact If wxETH drips when...

M-05 Unmitigated

Lines of code code-423n4/2023-05-xeth@aebc324L230 Vulnerability details If wxETH drips when nothing is staked, then the first staker can claim every drop. Mitigation code-423n4/2023-05-xeth@aebc324 This PR is added in the method accrueDrip to return if totalSupply == 0 to avoid dropping at 0. But...

It is not possible to execute actions that require ETH (or other protocol token)

Lines of code Vulnerability details Details Actions can have value attached to them. That means when action is being executed, a certain amount of ETH or other protocol token need to be sent by the caller with the contract call. This is why LlamaCore.executeAction is payable function...

Actions can stuck in Queued state

Lines of code Vulnerability details Impact According to current login - if action is in a Queued state, everyone may execute that action function executeActionActionInfo calldata actionInfo external payable is external without any modifier. When execution of action fails, it, however, is not bein...

The low-level call returns true if the address does not exist

Lines of code Vulnerability details Impact As written in the Solidity documentation , the call, delegatecall and staticcall low-level functions return true as its first return value if the account being invoked does not exist, as part of the design of the EVM. The existence of the account must be...

An attacker can steal funds from an Llama account by re-logging in.

Lines of code Vulnerability details Impact Funds are passed to LlamaAccount.execute and LlamaExecutor.execute using success, result = target.callvalue: msg.valuecallData However, there is no remedy for reentrancy. The target contract can steal funds from LlamaAccount when executed. Proof of Conce...

User ETH will be stacked on Executor contract if the target script doesn't handle ETH.

Lines of code Vulnerability details Description When user wants to execute an action, he must send ETH equal to the actionInfo.value, this ETH should be forwarded to executor.execute contract. Note: the function currently does't forward the ETH, it's explained on another issue 48, the fix is to...

Upgraded Q -> 2 from #327 [1686724891862]

Judge has assessed an item in Issue 327 as 2 risk. The relevant finding follows: L-04 addBid does not increment the endBlock of the auction when it is close to the end, preventing the protocol from capturing extra value When an Auction is created, it sets a lotItem.endBlock. This value remains...

Executing a script action with non-zero value results in frozen funds

Lines of code Vulnerability details Llama instances have a separate LlamaExecutor contract for action execution. When calling LlamaCore.executeAction, the flow is the following for simplicity, we ignore action guards: The function does internal validation: checking the current action state is...

Using transferFrom on ERC721 tokens transfer

Lines of code Vulnerability details Impact The transferFrom method is used instead of safeTransferFrom which is not recommended. It can result in loss of NFT if the address is not able to handle the received NFT. OpenZeppelin’s documentation discourages the use of transferFrom, use safeTransferFr...

createAction() ,castApproval(), castDisapproval() functions vulnerable replay attacks

Lines of code Vulnerability details Impact /// @notice Mapping of policyholders to function selectors to current nonces for EIP-712 signatures. /// @dev This is used to prevent replay attacks by incrementing the nonce for each operation createAction, /// castApproval and castDisapproval signed by...

expirationPeriod in the strategy contracts is not checked when calling LlamaCore.execute

Lines of code Vulnerability details Impact If executionPeriod is set to an extremely short time or even 0, then all actions can never be executed. Proof of Concept There are 7 states in an Action flow: Active, Canceled, Failed, Approved, Queued, Expired, Executed. Generally, the sequence goes lik...

THERE IS NO INPUT VALIDATION FOR CRITICAL STATE VARIABLES WHICH COULD BREAK THE CORE FUNCTIONALITY OF THE PROTOCOL

Lines of code Vulnerability details Impact In the LlamaAbsoluteStrategyBase.queuingPeriod variable denotes the minimum time in seconds between queueing and execution of action. The LlamaAbsoluteStrategyBase.minExecutionTime returns the block.timestamp + queuingPeriod timestamp. Here queuingPeriod...

LlamaExecutor#execute is not payable

Lines of code Vulnerability details Impact Since the execute function in LlamaExecutor.sol is not payable, nor the contract has the ability to receive ether, any action that requires sending ETH will eventually fail. Proof of Concept Tools Used Manual review Recommended Mitigation Steps One of th...

NATIVE TOKENS TRANSFERRED TO THE LlamaAccount CONTRACT CAN GET STUCK

Lines of code Vulnerability details Impact In the LlamaAccount contract there is a payable recieve to receive native tokens as shown below: receive external payable Hence this contrat accepts native tokens sent to this. But the problem is if any amount of native token is sent to this contract via...

ecrecover function is vulnerable to signature malleability

Lines of code Vulnerability details Impact Signature malleability potential exists which can be carried out by malicious actor to have two or more signatures that recover signer of same message. ecrecover function can be used to recover address by having signature and signed message hash to which...

Upgraded Q -> 2 from #130 [1686726021314]

Judge has assessed an item in Issue 130 as 2 risk. The relevant finding follows: Possible Infinite Loops If the condition triggers the continue, then the loop variable does not get incremented. The condition never changes, as the same condition is checked over and over again, resulting in an...

NATIVE TOKENS COULD GET STUCK INSIDE THE LlamaCore CONTRACT SINCE THERE IS NO WITHDRAWAL MECHANISM

Lines of code Vulnerability details Impact The LlamaCore contract has a single payable function LlamaCore.executeAction. It is used to execute actions by calling the executor.execute function. But the native token recieved by this contract is passed on to the executor.execute function call. Only...

ERC721 tokens can be forever locked when transferring

Lines of code Vulnerability details Impact An action that transfers ERC721 tokens from the account contract to another contract could result in the ERC721 tokens being locked forever if the receiving contract doesn't support receiving ERC721 tokens. Proof of Concept The LlamaAccount contract uses...

_newCastCount can overflow and lead to wrong state

Lines of code Vulnerability details Impact The method newCastCount only checks for overflow via the two parameters that are passed. If a user were to pass a quantity that is equal to typeuint128.max - currentCount + 1, they could effectively bring the newCastCount value to 0 or to any value they...

Clone LlamaCore and LlamaPolicy in LlamaFactory.sol may fail or DOS

Lines of code Vulnerability details Impact LlamaFactory uses Clones.cloneDeterministic to create new LlamaCore and LlamaPolicy contracts. The address of the new PrivatePool depends solely on the name parameter keccak256abi.encodePackedname provided by the administrator when calling the deploy...

In LlamaAccount.transferERC20(), Transaction revert if the Token does not support 0 value transfer

Lines of code Vulnerability details Impact In LlamaAccount.transferERC20, Transaction revert if the Token does not support 0 value transfer when transferring tokens to recipient address. In LlamaAccount.sol, transferERC20 function is given by, File: src/accounts/LlamaAccount.sol 165 function...

Blacklisted Address Can Exploit the Exchange

Lines of code Vulnerability details Impact Addresses that are blacklisted for popular ERC20 tokens such as USDC, USDT can be leveraged to exploit the exchange in a number of ways. These addresses cannot be liquidated in any case where they would be transferred back a leftover collateral amount in...

Manually assigning token IDs in ERC-721 can cause problems in the future

Lines of code Vulnerability details Impact 1. It can lead to duplicate token IDs. If two different people manually assign the same token ID to their tokens, this can create a conflict. This can lead to problems when trying to transfer or sell the tokens, as the recipient may not be able to claim...

Comparing Ether values using equality operators (== or !=) can be very hard to match each other . This will always reverts

Lines of code Vulnerability details Impact Comparing Ether values using equality operators can be very hard to match each other because Ether values are stored in a 256-bit unsigned integer uint256 in Solidity. This means that there are a very large number of possible Ether values, and it is very...

Gas griefing/thief in LlamaAccount execute()

Lines of code Vulnerability details Impact LlamaAccount execute function either delegatecall or call to the provided address. The address may contain malicious contract and should be treated as a malicious. This assumption was confirmed by implementation of this function and its comments...

Uses Send Value to transfer Native token instead of Transfer Function

Lines of code Vulnerability details Impact The Send Value is susceptible to a reentrancy attack. Proof of Concept If the recipient is a malicious contract that performs a reentrant call back into the LlamaAccount contract, it can execute code before the transfer is completed and potentially...

Transaction failed to deploy strategies

Lines of code Vulnerability details deployStrategies function in LlamaCore.sol takes the argument of array, It should put an upper bound on the length of the input, otherwise the transaction will run out of gas due to too much loop runs. Assessed type DoS --- The text was updated successfully, bu...

Mislead of policy holders due to wrong role description, Unwanted roles creation

Lines of code Vulnerability details Impact Proof of Concept initializeRoles is a public function so can be used by anyone to make the roles even if it is not wanted by the governance. updateRoleDescriptions is also a public function which can be changed by anyone hence changing the role descripti...

Execution does not work if the action has a non-zero value

Lines of code Vulnerability details Llama instances have a separate LlamaExecutor contract for action execution. When calling LlamaCore.executeAction, the flow is the following for simplicity, we ignore action guards: The function does internal validation: checking the current action state is...

Accidentally setting expirationPeriod to Zero will cause all actions execution to Fail.

Lines of code Vulnerability details Description During action execution, when user calls executeAction the function call getActionState to get the current state for the action, if this actions is queued successfully and the disapproval check passed, the function check if the action expired by...

Success value and msg.value not checked in llamaExecutor.sol

Lines of code Vulnerability details Impact Success value not checked. Result can fail silently. Msg.value can be lost. Proof of Concept Function execute does not check the validity of success. If execute is called and msg.value is greater than value, then excess msg.value will be stucked in...

Signature malleability in createActionBySig allow an attacker to replicate an attack

Lines of code Vulnerability details Overview The createActionBySig is part of a family of functions that can be called by signature. This functionality is shared with castApprovalBySig and castDisapprovalBySig. However, the last two functions rely on the preCastAssertion function that revert on...

Incorrect validation in LlamaRelativeQuorum results in improper approvals/disapprovals

Lines of code Vulnerability details Impact The LlamaRelativeQuorum contract is intended to allow for approvals/disapprovals to be configured such that, for example, an approval is triggered when a certain percentage of the total quantity for a role with this quantity being fixed at the creation o...

Role might be granted and revoked at the same block to manipulate the role supply and result in incorrect behavior of relative strategy

Lines of code Vulnerability details Impact There is a potential issue where a role can be granted, votes can be cast, and then the role can be revoked in the same block. This can lead to incorrect behavior of relative strategy in manipulating the vote supply. The problem arises because the role...

Unsafe delegatecall functionality can break core protocol functionality

Lines of code Vulnerability details Impact There are multiple contracts which include delegatecall functionality, including the execute function of the LlamaAccount contract and the execute function of the LlamaExecutor contract. The issue is that there's no controls, other than the standard role...

Attacker can revert execution by frontrunning "createAction"

Lines of code Vulnerability details Impact Malicious user can frontrun "createAction" to make action execution revert due to the "assertNoActionCreationsAtCurrentTimestamp" check Proof of Concept function assertNoActionCreationsAtCurrentTimestamp internal view if llamaExecutor == address0 return;...

[ H ] No Validation checks for "v" signature in castApprovalBySig

Lines of code Vulnerability details Impact V must correspond according to ECDSA principles values of either 27 or 28. Not checking this will result in an attacker crafting a malicious v value and bypassing any checks, withdrawing funds or accessing the function maliciously. Proof of Concept Craft...

potential reentrancy attack in _mint and _burn function

Lines of code Vulnerability details Impact potential reentrancy because state changes after minting and burning, should change state first Proof of Concept Tools Used Manual Review Recommended Mitigation Steps follow check-effect-interaction pattern as following: function mintaddress policyholder...

malicious policyholder can forbid setRoleHolder/revokeExpiredRole/revokePolicy calls

Lines of code Vulnerability details Impact All policyholders can't trigger functions related to setRoleHolder, such as setRoleHolder/revokeExpiredRole/revokePolicy. So malicious hackers can achieve DoS or use expired roles for a long unexpected time. Proof of Concept In...

Inconsistent Use of Error Handling in LlamaAccount Contract

Lines of code Vulnerability details Description: The LlamaAccount contract contains a bug that allows an attacker to execute arbitrary calls with the delegatecall opcode, which can lead to unintended consequences and potential security vulnerabilities. Steps to Reproduce: Deploy the LlamaAccount...

Missing Role Validation in LlamaAbsoluteStrategyBase.sol

Lines of code Vulnerability details Bug Description: The LlamaAbsoluteStrategyBase contract serves as a base contract for Llama strategies and implements various functions for action creation, approval, disapproval, and cancellation. However, it fails to validate the roles used in some critical...

The signature "V" length is not checked in castApprovalBySig, CastDisapprovalBySig & createActionBySig

Lines of code Vulnerability details Impact The length of the "v" value in the signatures is not checked. V must correspond according to ECDSA principles values of either 27 or 28. Not checking this will result in an attacker crafting a malicious v value and bypassing any checks, withdrawing funds...

LlamaRelativeQuorum isActionApproved / isActionDisapproved check condition error

Lines of code Vulnerability details Impact LlamaRelativeQuorum isActionApproved / isActionDisapproved check condition error: quantity holders. The two cannot be compared. In general quantity holder, so the approver was lower than expected. Proof of Concept diff --git...

[ H ] In executeAction function, ETH can remain stuck after reverting due to a failed call.

Lines of code Vulnerability details Impact If and when the executeAction function reverts at either of the checks, the amount of ETH sent along with the call will be locked in the contract forever, with no current measures of recovery. Proof of Concept executeAction is called, but the call fails ...

User can manipulate approvals and disapprovals of relative quorum strategy

Lines of code Vulnerability details Impact A malicious user with sufficient permissions can manipulate approvals and disapprovals of actions using the relative quorum strategy. They could effectively ensure that any action has an 100% chance of being approved or disapproved, even when the...

In LlamaRelativeQuorum, actionApprovalSupply/actionDisapprovalSupply can be changed anytime.

Lines of code Vulnerability details Impact The governance result might be manipulated seriously because the approval/disapproval supplies can be changed anytime by an attacker. Proof of Concept The LlamaRelativeQuorum uses approval/disapproval thresholds that are specified as percentages of total...

Arbitrary delegatecalls from LlamaAccount can be used to steal assets

Lines of code Vulnerability details Impact Using delegatecall to call arbitrary contracts is highly dangerous as it can be used to steal assets. An attacker could sneak in a contract that steals all the assets owned by the LlamaAccount contract. Proof of Concept Below is a diff to the existing...