Alvara Protocol security record

Impact:1. An attacker is able to atomically steal large amounts of the funding yield from the BasketTokenStandardPair contract. This is due to the fact that the mint function of the BasketTokenStandardPair contract will first issue the attacker shares based on the current reserve in the contract which is not the actual reserve balance in the contract, and then increase the reserve, which updates the reserve to the actual reserve balance in the contract. The Attacker is then able to withdraw their initial funds from the protocol, while also getting an equivalent amount of the funding yield which was just deposited into the contract (immediate profit from outstanding shares they gain atomically). 2. This is also susceptible to a flash loan attack. An attacker can exploit the vulnerability by executing flash loan transactions using both the contribute() and withdraw() functions. This allows the attacker to acquire large extra lp tokens and increase their share of assets within seconds, all without any meaningful contribution. The attacker only needs to obtain a flash loan of eth as the fund asset, and then they can use the contribute function to mint shares and subsequently call the withdraw function to receive the flash loan amount along with additional stolen lp tokens.

anyone can choose to change the smartfunds of any targetted BTS fund manager to bad or poor tokens and destroy the targetted BTS, affecting all of the contributions made on the BTS at zero cost to the attacker.

1. Any funds minted or deposited into btsPair contract and not used in that transaction can be stolen immediately2. Unintended behaviour as it expected to be interacted with only through bts for auto balancing logic but is bypassed

according to the documentation, “To successfully mint the BTS, the use must make a minimum contribution of 1 ETH (or 1 ETH equivalent in any currency). The minimum contribution is designed to prevent the frivolous, excessive creation of numerous SmartFunds’. In the contract implementation, this is not added to the createBTS function. Due to the failure to verify, users are at liberty to create numerous SmartFunds with amounts less than 1 ether.

the issue of index out of bounds arises when users invoke the primary createBTS function from the Factory contract in order to create new instance of the BTS and BTSPair. This issue disrupts the successful creation of the BTS because of the update being done to the _alvaIndex state variable. When the createFunction is triggered, it creates new instances of the BTS and BTSPair and afterwards invoke the initialize function from the BTS contract. Within the initialize function in BTS contract, the _alvaIndex variable is updated to the maximum of data type uint256 (Line 70) and also updated again (Line 94) in the  _checkValidTokensAndWeights function (Informational - to save gas, invoke once) but transaction would encounter index out of bounds within the _checkValidTokensAndWeights function when it reaches the if block (Line 113).  Supposing one of the token addresses passed is the Alva token address (Line 100), the _alvaIndex would change from the `type(uint256).max` to the index of the address in the ‘token[i]’ array, this will escape the index out of bounds error when it reaches Line 114 where _weight[_alvaIndex] < IFactory(factory)minPercentLVA() is meant to verify that the index should not be less than the minPercentALVA() read from the factory contract. However, if it is a case where none of the addresses is Alva token, _alvaIndex will be the maximum of type uint256 and on trying to know the location of _alvaIndex in _weight will cause the whole transaction to revert.

msg.value is attached multiple times to external swap calls and not compared with the actual total after swap iteration.

There is a difference between constant variables and immutable variables, and they should each be used in their appropriate contexts. While it doesn't save any gas because of the compiler handling, it's still best to use the right tool for the task at hand. Constants should be used for literal values written into the code, and immutable variables should be used for expressions or values calculated in or passed into the constructor.

With the use of modifiers on some functions restrict the privilege of who can call them. This gives us the certainty of some functions reverting when called by users not allowed to call this function, it is recommended to mark these functions as payable as this will save gas when they are called by privileged addresses.

the updateALVA function is primarily created to allow the owner of the Factory to update the Alva token address. The issue arises when in line 137, the if statement checks that the alva variable (a state variable already set at the initialize function) is not the null address. This check was intended to check that the input parameter passed by the contract owner is not a null address before it updates the alva state variable.

There are some functions in the contract that allows for the passage of address input values but fails to check that these addresses are not the null address. Although these functions are privileged functions allowed for the contract owner, it is important to safeguard setting some addresses to zero addresses.