Disallow ERC20._transfer from the zero address. (#1752)

* Add requirement of non-zero from to ERC20 transfer.

* Add test for transferFrom zero address to behavior.

* Create ERC20.transfer behavior.

* Add tests for _transfer.

* Add changelog entry.

* Fix linter error.

* Delete repeated test.

* Fix hardcoded error prefix.

* Update CHANGELOG.md

Co-Authored-By: Francisco Giordano <frangio.1@gmail.com>

* Address review comments.

CHANGELOG.md

@@ -12,6 +12,7 @@
 
 ### Bugfixes:
  * `PostDeliveryCrowdsale`: some validations where skipped when paired with other crowdsale flavors, such as `AllowanceCrowdsale`, or `MintableCrowdsale` and `ERC20Capped`, which could cause buyers to not be able to claim their purchased tokens. ([#1721](https://github.com/OpenZeppelin/openzeppelin-solidity/pull/1721))
+ * `ERC20._transfer`: the `from` argument was allowed to be the zero address, so it was possible to internally trigger a transfer of 0 tokens from the zero address. This address is not a valid destinatary of transfers, nor can it give or receive allowance, so this behavior was inconsistent. It now reverts. ([#1752](https://github.com/OpenZeppelin/openzeppelin-solidity/pull/1752))
 
 ## 2.2.0 (2019-03-14)
 

contracts/mocks/ERC20Mock.sol

@@ -20,6 +20,10 @@ contract ERC20Mock is ERC20 {
         _burnFrom(account, amount);
     }
 
+    function transferInternal(address from, address to, uint256 value) public {
+        _transfer(from, to, value);
+    }
+
     function approveInternal(address owner, address spender, uint256 value) public {
         _approve(owner, spender, value);
     }

contracts/token/ERC20/ERC20.sol

@@ -125,6 +125,7 @@ contract ERC20 is IERC20 {
      * @param value The amount to be transferred.
      */
     function _transfer(address from, address to, uint256 value) internal {
+        require(from != address(0), "ERC20: transfer from the zero address");
         require(to != address(0), "ERC20: transfer to the zero address");
 
         _balances[from] = _balances[from].sub(value);

test/token/ERC20/ERC20.test.js

@@ -3,6 +3,7 @@ const { ZERO_ADDRESS } = constants;
 
 const {
   shouldBehaveLikeERC20,
+  shouldBehaveLikeERC20Transfer,
   shouldBehaveLikeERC20Approve,
 } = require('./ERC20.behavior');
 
@@ -330,6 +331,20 @@ contract('ERC20', function ([_, initialHolder, recipient, anotherAccount]) {
     });
   });
 
+  describe('_transfer', function () {
+    shouldBehaveLikeERC20Transfer('ERC20', initialHolder, recipient, initialSupply, function (from, to, amount) {
+      return this.token.transferInternal(from, to, amount);
+    });
+
+    describe('when the sender is the zero address', function () {
+      it('reverts', async function () {
+        await shouldFail.reverting.withMessage(this.token.transferInternal(ZERO_ADDRESS, recipient, initialSupply),
+          'ERC20: transfer from the zero address'
+        );
+      });
+    });
+  });
+
   describe('_approve', function () {
     shouldBehaveLikeERC20Approve('ERC20', initialHolder, recipient, initialSupply, function (owner, spender, amount) {
       return this.token.approveInternal(owner, spender, amount);