fix: audit updates (yearn#284)

* fix: do not issue zero shares (yearn#281)

* fix: prevent setting wrong reward address (yearn#279)

* fix: prevent rewards address to 0x0 or self

* fix: test tests/functional/vault/test_config.py

Co-authored-by: Just some guy <3859395+fubuloubu@users.noreply.github.com>

Co-authored-by: Just some guy <3859395+fubuloubu@users.noreply.github.com>

* fix: check erc20 decimals (yearn#283)

* fix: initialize function does not validate ERC20 decimals

* fix: use cached decimals

* fix: shares can be indirectly transferred to 0x0 (yearn#277)

Co-authored-by: Just some guy <3859395+fubuloubu@users.noreply.github.com>

contracts/Vault.vy

@@ -283,9 +283,11 @@ def initialize(
         self.symbol = concat("yv", DetailedERC20(token).symbol())
     else:
         self.symbol = symbolOverride
-    self.decimals = DetailedERC20(token).decimals()
-    if self.decimals < 18:
-      self.precisionFactor = 10 ** (18 - self.decimals)
+    decimals: uint256 = DetailedERC20(token).decimals()
+    self.decimals = decimals
+    assert decimals < 256 # dev: see VVE-2020-0001
+    if decimals < 18:
+      self.precisionFactor = 10 ** (18 - decimals)
     else:
       self.precisionFactor = 1
 
@@ -440,6 +442,7 @@ def setRewards(rewards: address):
     @param rewards The address to use for collecting rewards.
     """
     assert msg.sender == self.governance
+    assert not (rewards in [self, ZERO_ADDRESS])
     self.rewards = rewards
     log UpdateRewards(rewards)
 
@@ -624,7 +627,7 @@ def _transfer(sender: address, receiver: address, amount: uint256):
     # See note on `transfer()`.
 
     # Protect people from accidentally sending their shares to bad places
-    assert not (receiver in [self, ZERO_ADDRESS])
+    assert receiver not in [self, ZERO_ADDRESS]
     self.balanceOf[sender] -= amount
     self.balanceOf[receiver] += amount
     log Transfer(sender, receiver, amount)
@@ -804,6 +807,7 @@ def _issueSharesForAmount(to: address, amount: uint256) -> uint256:
     else:
         # No existing shares, so mint 1:1
         shares = amount
+    assert shares != 0 # dev: division rounding resulted in zero
 
     # Mint new shares
     self.totalSupply = totalSupply + shares
@@ -848,6 +852,7 @@ def deposit(_amount: uint256 = MAX_UINT256, recipient: address = msg.sender) ->
     @return The issued Vault shares.
     """
     assert not self.emergencyShutdown  # Deposits are locked out
+    assert recipient not in [self, ZERO_ADDRESS]
 
     amount: uint256 = _amount
 

tests/functional/vault/test_config.py

@@ -4,6 +4,7 @@
 import pytest
 import brownie
 
+from brownie import ZERO_ADDRESS
 
 PACKAGE_VERSION = yaml.safe_load(
     (Path(__file__).parent.parent.parent.parent / "ethpm-config.yaml").read_text()
@@ -203,3 +204,11 @@ def test_vault_setLockedProfitDegration_range(gov, vault):
     vault.setLockedProfitDegration(DEGREDATION_COEFFICIENT, {"from": gov})
     with brownie.reverts():
         vault.setLockedProfitDegration(DEGREDATION_COEFFICIENT + 1, {"from": gov})
+
+
+def test_vault_setParams_bad_vals(gov, vault):
+    with brownie.reverts():
+        vault.setRewards(ZERO_ADDRESS, {"from": gov})
+
+    with brownie.reverts():
+        vault.setRewards(vault, {"from": gov})

tests/functional/vault/test_shares.py

@@ -32,6 +32,15 @@ def test_deposit_with_wrong_amount(vault, token, gov):
         vault.deposit(balance, {"from": gov})
 
 
+def test_deposit_with_wrong_recipient(vault, token, gov):
+    balance = token.balanceOf(gov)
+    token.approve(vault, 2 ** 256 - 1, {"from": gov})
+    with brownie.reverts():
+        vault.deposit(
+            balance, "0x0000000000000000000000000000000000000000", {"from": gov}
+        )
+
+
 def test_deposit_with_guest_list(vault, guest_list, token, gov, rando, history):
     # Make sure we're attempting to deposit something
     token.transfer(rando, token.balanceOf(gov) // 2, {"from": gov})
@@ -296,3 +305,12 @@ def test_transferFrom(accounts, token, vault):
 
     assert vault.balanceOf(a) == 0
     assert vault.balanceOf(b) == token.balanceOf(vault)
+
+
+def test_do_not_issue_zero_shares(gov, token, vault):
+    token.approve(vault, 500, {"from": gov})
+    vault.deposit(500, {"from": gov})
+    token.transfer(vault, 500)  # inflate price
+    assert vault.pricePerShare() == 2 * 10 ** token.decimals()  # 2:1 price
+    with brownie.reverts():
+        vault.deposit(1, {"from": gov})