Merge pull request #2213 from nventuro/merge-v2.5.1

Merge v2.5.1 into v3.0.0

CHANGELOG.md

@@ -1,5 +1,10 @@
 # Changelog
 
+## 3.0.1 (unreleased)
+
+### Bugfixes
+ * `ERC777`: fixed the `_approve` internal function not validating some of their arguments for non-zero addresses. ([#2213](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2213))
+
 ## 3.0.0 (2020-04-20)
 
 ### New features
@@ -37,6 +42,11 @@
  * `ERC20`: added a constructor for `name` and `symbol`. `decimals` now defaults to 18. ([#2161](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2161))
  * `Strings`: renamed `fromUint256` to `toString` ([#2188](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2188))
 
+## 2.5.1 (2020-04-24)
+
+### Bugfixes
+ * `ERC777`: fixed the `_send` and `_approve` internal functions not validating some of their arguments for non-zero addresses. ([#2212](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2212))
+
 ## 2.5.0 (2020-02-04)
 
 ### New features

contracts/mocks/ERC777Mock.sol

@@ -22,4 +22,8 @@ contract ERC777Mock is Context, ERC777 {
     ) public {
         _mint(to, amount, userData, operatorData);
     }
+
+    function approveInternal(address holder, address spender, uint256 value) public {
+        _approve(holder, spender, value);
+    }
 }

contracts/token/ERC777/ERC777.sol

@@ -415,10 +415,13 @@ contract ERC777 is Context, IERC777, IERC20 {
         emit Transfer(from, to, amount);
     }
 
+    /**
+     * @dev See {ERC20-_approve}.
+     *
+     * Note that accounts cannot have allowance issued by their operators.
+     */
     function _approve(address holder, address spender, uint256 value) internal {
-        // TODO: restore this require statement if this function becomes internal, or is called at a new callsite. It is
-        // currently unnecessary.
-        //require(holder != address(0), "ERC777: approve from the zero address");
+        require(holder != address(0), "ERC777: approve from the zero address");
         require(spender != address(0), "ERC777: approve to the zero address");
 
         _allowances[holder][spender] = value;

test/token/ERC777/ERC777.test.js

@@ -1,6 +1,7 @@
 const { accounts, contract, web3 } = require('@openzeppelin/test-environment');
 
-const { BN, expectEvent, expectRevert, singletons } = require('@openzeppelin/test-helpers');
+const { BN, constants, expectEvent, expectRevert, singletons } = require('@openzeppelin/test-helpers');
+const { ZERO_ADDRESS } = constants;
 
 const { expect } = require('chai');
 
@@ -15,6 +16,7 @@ const {
 
 const {
   shouldBehaveLikeERC20,
+  shouldBehaveLikeERC20Approve,
 } = require('../ERC20/ERC20.behavior');
 
 const ERC777 = contract.fromArtifact('ERC777Mock');
@@ -40,7 +42,23 @@ describe('ERC777', function () {
       this.token = await ERC777.new(holder, initialSupply, name, symbol, defaultOperators);
     });
 
-    shouldBehaveLikeERC20('ERC777', initialSupply, holder, anyone, defaultOperatorA);
+    describe('as an ERC20 token', function () {
+      shouldBehaveLikeERC20('ERC777', initialSupply, holder, anyone, defaultOperatorA);
+
+      describe('_approve', function () {
+        shouldBehaveLikeERC20Approve('ERC777', holder, anyone, initialSupply, function (owner, spender, amount) {
+          return this.token.approveInternal(owner, spender, amount);
+        });
+
+        describe('when the owner is the zero address', function () {
+          it('reverts', async function () {
+            await expectRevert(this.token.approveInternal(ZERO_ADDRESS, anyone, initialSupply),
+              'ERC777: approve from the zero address'
+            );
+          });
+        });
+      });
+    });
 
     it.skip('does not emit AuthorizedOperator events for default operators', async function () {
       expectEvent.not.inConstructor(this.token, 'AuthorizedOperator'); // This helper needs to be implemented