Audit Acknowledgements

Auditors look through a different lens than protocol designers and creators. BetterBank's team may for example want to use a certain feature in a safe way, but then the auditor marks the feature itself as dangerous if used in a different way. That's their job as auditors and we commend them for being so thorough, but it does paint our system as less safe than it really is.

It is is a bit like transporting an unloaded gun and ammunition to a shooting range safely, with the auditor then coming in to point out that if we'd load the gun during transport, we'd create a safety hazard. We know that, which is why we only load it while at the shooting range and use it as intended. The way we use the gun, or in this case the code, is what makes it safe.

To be completely transparent, in this chapter we'll outline all issue acknowledgements in the audit and outline why we consider it safe due to how we use that code. Based on that, anyone who intents to use BetterBank's platform can make up their own mind about whether it's safe or not.

For your reading convenience, we copy-paste first Zokyo's explanation of the issue plus their idea of the impact and offered recommendation, and then secondly our response to them. If we feel like we want to explain a little bit more to the public, we state that lafter these two texts.

Issue 5, Medium 2

The excluding from the total supply feature can be bypassed

Description: The addExcludedAddress() function within the FavorTreasury.sol smart contract seems to be used to exclude certain amount of funds from the total supply. However, if funds are transferred to a new address, they will not be excluded.

Impact: If it is wanted to exclude 100 tokens so that an address which holds 100 tokens is excluded and later the excluded address transfers the 100 tokens to a new non excluded one, they will be accounted again.

Recommendation: Instead of excluding addresses from the total supply, exclude the amount of tokens.

Our response to Zokyo: This feature is only intended to be used to exclude arbitrary balances of tokens from chain burn/dead addresses or in the event tokens become stuck in contracts.

While currently we only target official burn addresses to exclude burnt tokens from supply, we might in the future use this feature for an active supply contraction mechanism

Extra explanation: We have no desire to use this feature to exclude a set amount of tokens, or even to target active wallets. We target burn wallets and maybe we'll target contracts that for whatever reason got some Favor tokens stuck in them, so that the tokens in those wallets won't be counted as supply for our Groves printing.

The one and only moment that this would result in a situation where tokens in excluded wallets would actually come back into counted supply is when such excluded wallets hold LP's or other wrappers that can be interacted with to release tokens without action from the excluded wallet itself.

That would mean that someone would, at 100% cost to themself, have sent Favor LP to a burn wallet. Then some Favor tokens would enter and leave the burn wallet.

We don't see this as an issue though, because we're intending to use that very feature in the near future to create an additional elastic supply mechanism by targeting the treasury wallet that holds the LP Cushion as excluded.

Upon liquidations the system breaks LP and sells into the LP that's left, which results in many Favor tokens ending up in the LP Cushion. Excluding the Favor in the LP Cushion from supply for means of calculating Grove printing until they get bought out of that LP Cushion again is actually a healthy mechanism of deflation and inflation responding to demand.

This is why we acknowledge this issue as an intended feature.

Issue 8, Medium 5

Incorrect Percentage Calculation If Quote Token Is Non-18 Decimal Token

Description: In the FavorTreasury while allocating seigniorage the percentage of price is compared with the minSupplyExpansionPercentage and the maxSupplyExpansionPercentage both of which are expected to be basis points of the percentage (after being multiplied by 1e13) , but if the quote token in the oracle is a non-18 decimal token like USDC/USDT then the price of the favor token (getFavorPrice()) would be in USDC/USDT i.e 6 decimals and the comparison with min and max would almost always result in percentage being less than min.

Impact: In the FavorTreasury while allocating seigniorage the percentage of price is compared with the minSupplyExpansionPercentage and the maxSupplyExpansionPercentage both of which are expected to be basis points of the percentage (after being multiplied by 1e13) , but if the quote token in the oracle is a non-18 decimal token like USDC/USDT then the price of the favor token (getFavorPrice()) would be in USDC/USDT i.e 6 decimals and the comparison with min and max would almost always result in percentage being less than min.

Recommendation: Take into account the decimal of the token while calculating the percentage.

Our response to Zokyo: In FavorTreasury getFavorPrice() is only ever used to quote the TWAP of Favor tokens which are always deployed with standard 18 decimals.

Extra explanation: The FavorTreasury is only used to quote and calculate expansion of Favor tokens and those are always in a decimal 18 format, because they're tokens that the BetterBank team has specifically designed for use in the BetterBank system.

Since incompatible tokens will never be used for the Groves contracts, we acknowledge this issue as inconsequential.

Issue 13, Low 1

A Malicious User Can Utilise Flash Loans In Order To Get Free Tokens By Exploiting The Qualified Favor Bonus

Description: The UniswapWrapper allows users to trade their tokens in order to receive Favor and vice versa except trading the opposite way does not qualify for bonuses. A malicious user can take out a large flash loan, trade for favor and favor bonus (tanking the favor price) then trade back to the other token. Because the price is significantly lower than previously, the bonus qualified and obtained now has more impact when trading back allowing a malicious user to extract value from the uniswap pools through the favor bonus feature.

Proof of Concept: The proof of concept below shows a user taking out a large flash loan, makes a trade, then trades back resulting in the extraction of value from the uniswap pool using qualified bonuses:

https://gist.github.com/chris-zokyo/8951f4832b975bdc494a9990baa84763

Impact: A malicious user with a flashloan or a whale can extract value from the uniswap pools through the wrapper by exploiting the bonuses feature of the FavorPls contract.

Recommendation: It’s recommended that users need to wait a certain amount of time before being able to claim bonuses if they are to be awarded during trades. Client comment:

Our response to Zokyo: (All sells of) Favor have a 50% sell tax, all buys through the router wrapper are given 44% bonus in Esteem (50% in original favor contract*, now updated to 44%) which has no liquidity and can only be redeemed for 70% of its value in Favor. For example, not accounting for slippage, user flashloans $1m buys favor, receives $440k bonus in Esteem token, redeems this Esteem token to Favor for a value of $308k then sells all the Favor (worth 1,308k including the bonus) with a 50% sell tax for $654k and is not able to pay back the flashloan. Even when not selling, but instead borrowing against the Favor, the yield on 1,308k Favor is $784,800k, which is also not enough to pay back the flashloan. *Even in the old setting, the yield sold/ borrowed would be $675,000 / $810,000, which is also far removed from the $1.5M required to pay back the flash loan.

Extra Explanation: NB: A large part of our answer was cut off in the final report, which is why we added (all sells of) at the beginning of the sentence to restore the syntax. Also, there has been much discussion about this issue, as after our above answer, it was maintained that the problem could persist if the malicious user would sell over a non-sanctioned LP, that we're not taxing.

As explained in our answer, the exploit will not work over any of our official pairs, since they levy tax. In addition, for all of BetterBank's system, we exclusively draw our Favor price feed from the official pair. Any unsanctioned pairs will not affect the price feed for BetterBank's system.

If anyone acquires Favor and builds their own unsanctioned LP with it or would alternatively list it on a CEX, then they do so with their own assets. Neither BetterBank nor any users using BetterBank as intended will have allocated assets in this way. Such an unsanctioned LP will, because of how arbitrage works, likely have a Favor price that fluctuates between 50% and 60% of our official price. As long as this is the case, the exploit will not work, because the Favor's value cannot be recouped.

For all of BetterBank's system, we exclusively draw our Favor price feed from the official pair. Any unsanctioned pairs will not affect the price feed for BetterBank's system. But someone might write an oracle that just takes our official price and apply it, to the dire detriment of anyone committing funds to that market pair. In this case the exploit would indeed work, but the duped party would not be BetterBank nor any user who uses BetterBank as intended.

Since neither BetterBank nor users who use BetterBank as intended are in danger, we acknowledge this issue as inconsequential. In addition, we see this possible issue as a cautionary tale for anyone planning to use any Favor token in unintended ways.

Issue 19, low 7

High probabilities of grove amounts being incorrectly calculated

Description: The allocateSeigniorage() function within the FavorTreasury.sol smart contract executes the following operation to calculate the amount of funds which are sent to the grove:

uint256 _savedForGrove = (favorSupply * _percentage) / 1e18 / 24; // divide by 24 for each epoch per day

There is a division by 24 expecting each day having increased 24 epochs. However, epochs are only increased if allocateSeigniorage() is executed.

Impact: If the function allocateSeigniorage() is not executed exactly 1 time each hour in a really precisely way, there will not be an increase of 24 epochs in 24 hours, so the assumption will not be correct.

Recommendation: Refactor how the operation is performed.

Our response to Zokyo: The AllocateSeigniorage() function is called via a keeper bot running a cron job on the hour every hour, expansion is assumed to increase per epoch rather than exactly per hour, so this is within assumptions. If the keeper bot fails, this function can be called by any EOA onchain. We will implement a button in the UI so that if ever the bot fails to call, then after the hour has passed, any user can call it without having to go into the contract.

Extra Explanation: While BetterBank's seigniorage system should experience one epoch per hour, nothing about the system is in peril if that doesn't happen. The only thing that happens is that stakers in the Groves miss out on 1/24th of their daily Favor printing. Since anyone (especially WIldlanders waiting for their Favor allocation) is able to call the AllocateSeigniorage() function if our keeper bot fails, we believe to have sufficient redundancy, considering the harmlessness of a missed call.

Issue 22, low 10

Twap Function Might Return Incorrect Prices

Description: In FavorTreasury the getFavorUpdatedPrice can be used to get the price of one favor token, if the timeElapsed is small (the difference between current timestamp and the latest timestamp when an update was made) then the twap price returned is more probable to be incorrect and prone to manipulation.

Impact: twap() function might return stale/manipulated price.

Recommendation: It should be acknowledged to the users using this functionality offchain that this price might be incorrect.

Our response to Zokyo: The TWAP function is not used in the contract and is only referenced in views for UI. A comment was added to the contracts referencing it and will also be made on the UI, where referenced, to ensure users are aware of the more variable nature of its price calculations.

Extra explanation: We offer this feature to provide Wildlanders with a live feed in the UI of what the TWAP will be if the price stays as it currently is. We consider such information to be important for people who play the Wildlands.

Issue 23, low 11

Owner Can Assign Unlimited Minting Privileges

Description: The Esteem ERC20 contract allows the owner to grant or revoke minting privileges through the addMinter and removeMinter functions. Minting is strictly controlled by addresses set as minters. However, there's no limitation, timelock, or governance process on granting minter status. The owner can assign minting rights arbitrarily and instantly to any address.

Impact: This presents significant centralization risk: a compromised owner account or malicious behavior by the owner could grant minting privileges to a malicious actor or untrusted contract, enabling unlimited minting and dilution of token supply.

Proof of Concept: - The owner’s wallet or key management is compromised. - The attacker (or malicious owner) calls addMinter(attackerAddress) to grant themselves minter status. - The attacker then calls mint() repeatedly, creating an unlimited amount of Esteem tokens. - The newly minted tokens are dumped onto the market, rapidly reducing the token's price and trust, and allowing the attacker to profit significantly.

Recommendation: - Restrict minting privileges only to trusted, audited contracts or a multi-sig controlled minter management process. - Implement a two-step ownership transfer . - Add a timelock mechanism or DAO governance vote for adding or removing minters, ensuring the community can detect and prevent unauthorized minting before it happens. - Consider implementing a hard-coded maximum supply cap to prevent unlimited minting, providing an additional safeguard against severe token inflation. - Implement monitoring on-chain events (MinterAdded, MinterRemoved, and Mint) so suspicious or unusual activity is flagged quickly.

Our response to Zokyo: The suggestion of a multisig ownership as well as a timelock mechanism will be implemented. On top of that, off-chain security measures will be taken to protect the controlling wallets of that multisig. A hard-coded maximum supply cap would fundamentally invalidate the intended design.

Extra explanation: We require this feature to assign minting privileges to certain contracts when we list new tokens on our platform. While using a secure multisig for this goes without saying, we liked the suggestion of a 24 hour time-lock and an on-chain event management system. That way people can always check if what happens on-chain is congruent with what we announced. To hard-code a supply cap however really won't work with BetterBank's system.

Issue 24, low 12

Unauthorized Minting Risk via Unrestricted isMinter Role Assignment Description: The FavorPLS contract allows the contract owner to grant minting privileges to any address through the addMinter(address account) function without additional security controls.

Impact: This unrestricted ability to assign the minter role introduces a significant centralization risk, potentially enabling malicious or compromised addresses to mint an unlimited number of FavorPLS tokens, diluting token value and harming legitimate token holders. Proof of Concept: - The contract owner calls FavorPLS:addMinter(maliciousAddress). - The malicious address gains unrestricted minting capabilities. - The attacker calls FavorPLS:mint(recipient, amount) repeatedly, creating tokens at will.

Recommendation: Implement additional access controls and security checks, such as a multi-signature mechanism, timelock, or community governance approval before assigning minting privileges. This ensures no single address or individual can unilaterally gain control of minting capabilities, mitigating centralization and unauthorized minting risks. Our response to Zokyo: The suggestion of a multisig ownership as well as a timelock mechanism will be implemented. On top of that, off-chain security measures will be taken to protect the controlling wallets of that multisig. A hard-coded maximum supply cap would fundamentally invalidate the intended design.

Extra Explanation: Since this issue is virtually identical to the previous one, we give the same answer.

Issue 27, low 15

Farm-and-Run Seigniorage Exploit via Immediate Stake-and-Withdraw

Description: An attacker can game the seigniorage distribution by staking a minimal amount of tokens immediately before the treasury’s allocateSeigniorage() call, collecting their pro-rata share of freshly minted rewards, and then withdrawing their stake in the same epoch. Because there is no lockup or snapshot window, they can repeat this every epoch at near-zero cost.

Impact: The protocol’s reward mechanism can be drained by “flash” stakes of arbitrarily small size. An attacker staking 10 FAVOR into a 10 000 FAVOR pool could mint ≈1 FAVOR per epoch for free, repeatedly, diluting legitimate stakers and draining seigniorage over time. Recommendation: Disallow withdrawal or reward claims for stakes made within the current epoch.

Our response to Zokyo: Favor is not staked in the Groves. Esteem is staked in the Groves. Since AllocateSeigniorage() is called for allocation of all Favor types in one block, there is no possibility for Esteem stakers to move their Esteem to another Favor Grove in time to receive that Grove’s Favor as well, which if it were possible would be the only benefit to farm-and-run. Since Esteem cannot be sold at full value, anyone trying to buy-farm-sell would incur serious losses. If users wish to spend the gas for staking and unstaking, they’re welcome to do so as often as they want between the calls for AllocateSeigniorage().

It is true that if users would suddenly flood an otherwise sparsely staked grove with Esteem right before AllocateSeigniorage(), the pool allocation per Esteem would be diluted for everyone. This is however an intended part of the strategy game that is the Wildlands.

Issue 30, low 18

View functions may return stale prices

Description: There are several view functions which call .consult() without first calling update(), for example: getFavorPrice().

Impact: If update() is not called, they may return a stale price.

Recommendation: Call update() before calling consult().

Our response to Zokyo: In FavorTreasury the favor oracle is updated every hour during the AllocateSeigniorage call, after this update getFavorPrice() view is used to determine the current TWAP price for printing. The oracle utilizes this 1hour period to compute the price average which is important for all minting and redeeming functions on all contracts however should only be updated once per hour to compute an average over that time and not on a shorter time period. Updating the oracle every hour using keepers ensures that the price average remains fresh and with a sufficient TWAP for all functions that use consult().

Issue 35, informational 1

Esteem and Favor PLS tokens can get minted with unlimited supply

Description: The Esteem.sol smart contract implements a mint() function which allows minters to mint tokens without a limited supply.

function mint(address recipient_, uint256 amount_) public onlyMinter {

mint(recipient, amount_);

}

The same situation applies to the favorPLS.sol smart contract:

function mint(address recipient_, uint256 amount_) external {

require(isMinter[msg.sender], "Not authorized to mint");

mint(recipient, amount_);

}

Our response to Zokyo: We don't intend to have supply limits on Favor or Esteem tokens as the system is designed to expand perpetually, and faster or slower based on demand. While Esteem has a contractive function through the smelter, Favor does not and this is intentional. In times of low demand, the low price of Favor will create effective value contraction without supply contraction. Since Favor is primarily a collateral asset, this is enough.

Extra explanation: Even though here we state to have no contractive supply on Favor, we are actually working on that.

Issue 36, informational 2

If epoch is not correctly updated within the treasury it will lead to staking features not working as expected

Description: The main functions within the Staking.sol smart contract relies on the epoch variable from the Treasury.sol smart contract. If the variable is not updated correctly within the treasury, the functionalities in Staking will not work as expected.

Recommendation: Make sure to call and often update and correctly the epoch variable within the treasury contract.

Our response to Zokyo: We use a keeper bot to call function AllocateSeigniorage() on the FavorTreasury contract each hour which will update the TWAP price of the Favor token in the Oracle contract and also update the epoch. If the keeper bot fails, this function can be called by any EOA onchain. We will implement a button in the UI so that if ever the bot fails to call, then after the hour has passed, any user can call it without having to go into the contract.

Extra explanation: We feel that this is a third way way of writing down the situation that was already presented as issues 23 and 24.

Issue 37, informational 3

Centralized behavior

Description:

  • `logBuy()`: this function which can only be called by `isBuyWrapper` users can be called at any time, any times as wanted and the `amount` parameter can be set to any value, leading to the inflation of `pendingBonus` for `user` and the mint of esteem tokens for the treasury.

  • `PulseMinter.sol`: admin can withdraw any amount of tokens and ether from the contract.

Our response to Zokyo:

logBuy() - We want to reward any user buy of Favor with Esteem as long as the Favor has a TWAP price of lower than 3.5x its paired asset, and doing so through the wrapper allows us to control this best. We have no intention on limiting Esteem minting bonus/supply for users.

PulseMinter.sol - We have implemented an admin withdraw function for tokens or PLS in the contract only for purposes of recovering stuck tokens mistakenly sent to the contract. Essentially this contract holds no tokens for users. All tokens are immediately transferred in and out instantly on all functions, so the only time a token would be left in the contract is from a mistaken transfer by a user, in which case these functions allow us to recover the tokens for users.

Last updated