Skip to content

Commit f3c6c4f

Browse files
pedromauri-rsk
pedro
authored and
mauri-rsk
committed
Non authorized contracts can call the Bridge
Following the audit report we will allow any contract to call the bridge. We will remove the mapping allowedContracts from AllowTokens and won't use checkWhitelisted in the Bridge
1 parent 0ea2831 commit f3c6c4f

File tree

5 files changed

+2
-86
lines changed

5 files changed

+2
-86
lines changed

‎bridge/contracts/AllowTokens.sol‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ contract AllowTokens is Initializable, UpgradableOwnable, UpgradableSecondary, I
1414

1515
address constant private NULL_ADDRESS = address(0);
1616
uint256 constant public MAX_TYPES = 250;
17-
mapping (address => bool) public allowedContracts;
1817
mapping (address => TokenInfo) public allowedTokens;
1918
mapping (uint256 => Limits) public typeLimits;
2019
address public bridge;
@@ -25,8 +24,6 @@ contract AllowTokens is Initializable, UpgradableOwnable, UpgradableSecondary, I
2524

2625
event SetToken(address indexed _tokenAddress, uint256 _typeId);
2726
event AllowedTokenRemoved(address indexed _tokenAddress);
28-
event AllowedContractAdded(address indexed _contractAddress);
29-
event AllowedContractRemoved(address indexed _contractAddress);
3027
event TokenTypeAdded(uint256 indexed _typeId, string _typeDescription);
3128
event TypeLimitsChanged(uint256 indexed _typeId, Limits limits);
3229
event UpdateTokensTransfered(address indexed _tokenAddress, uint256 _lastDay, uint256 _spentToday);
@@ -136,16 +133,6 @@ contract AllowTokens is Initializable, UpgradableOwnable, UpgradableSecondary, I
136133
return typeDescriptions[index];
137134
}
138135

139-
function addAllowedContract(address _contract) external notNull(_contract) onlyOwner {
140-
allowedContracts[_contract] = true;
141-
emit AllowedContractAdded(_contract);
142-
}
143-
144-
function removeAllowedContract(address _contract) external notNull(_contract) onlyOwner {
145-
allowedContracts[_contract] = false;
146-
emit AllowedContractRemoved(_contract);
147-
}
148-
149136
function isTokenAllowed(address token) public view notNull(token) returns (bool) {
150137
return allowedTokens[token].allowed;
151138
}

‎bridge/contracts/Bridge.sol‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -198,24 +198,16 @@ contract Bridge is Initializable, IBridge, IERC777Recipient, UpgradablePausable,
198198
*/
199199
function receiveTokensTo(address tokenToUse, address to, uint256 amount) public {
200200
address sender = _msgSender();
201-
checkWhitelisted(sender);
202201
//Transfer the tokens on IERC20, they should be already Approved for the bridge Address to use them
203202
IERC20(tokenToUse).safeTransferFrom(sender, address(this), amount);
204203
crossTokens(tokenToUse, sender, to, amount, "");
205204
}
206205

207-
function checkWhitelisted(address sender) private view {
208-
if (sender.isContract()) {
209-
require(allowTokens.allowedContracts(sender), "Bridge: from contract not whitelisted ");
210-
}
211-
}
212-
213206
/**
214207
* Use network currency and cross it.
215208
*/
216209
function depositTo(address to) external payable {
217210
address sender = _msgSender();
218-
checkWhitelisted(sender);
219211
require(address(wrappedCurrency) != NULL_ADDRESS, "Bridge: wrappedCurrency empty");
220212
wrappedCurrency.deposit.value(msg.value)();
221213
crossTokens(address(wrappedCurrency), sender, to, msg.value, "");
@@ -238,8 +230,6 @@ contract Bridge is Initializable, IBridge, IERC777Recipient, UpgradablePausable,
238230
require(to == address(this), "Bridge: Not to this address");
239231
address tokenToUse = _msgSender();
240232
require(erc1820.getInterfaceImplementer(tokenToUse, _erc777Interface) != NULL_ADDRESS, "Bridge: Not ERC777 token");
241-
//This can only be used with trusted contracts
242-
checkWhitelisted(from);
243233
require(allowTokens.isTokenAllowed(tokenToUse), "Bridge: token not allowed");
244234
require(userData.length != 0 || !from.isContract(), "Bridge: Specify receiver address in data");
245235
address receiver = userData.length == 0 ? from : Utils.bytesToAddress(userData);

‎bridge/contracts/IAllowTokens.sol‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,6 @@ interface IAllowTokens {
2828
uint256 typeId;
2929
}
3030

31-
function allowedContracts(address sender) external view returns (bool);
32-
3331
function version() external pure returns (string memory);
3432

3533
function getInfoAndLimits(address token) external view returns (TokenInfo memory info, Limits memory limit);

‎bridge/test/AllowTokens_test.js‎

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -437,39 +437,6 @@ contract('AllowTokens', async function (accounts) {
437437

438438
});
439439

440-
describe('AllowedContracts', async function() {
441-
442-
it('should addAllowedContract', async function() {
443-
assert.equal(false, (await this.allowTokens.allowedContracts(anotherAccount)));
444-
await this.allowTokens.addAllowedContract(anotherAccount, { from: manager });
445-
assert.equal(true, (await this.allowTokens.allowedContracts(anotherAccount)));
446-
});
447-
448-
it('should removeAllowedContract', async function() {
449-
await this.allowTokens.addAllowedContract(anotherAccount, { from: manager });
450-
assert.equal(true, (await this.allowTokens.allowedContracts(anotherAccount)));
451-
await this.allowTokens.removeAllowedContract(anotherAccount, { from: manager });
452-
assert.equal(false, (await this.allowTokens.allowedContracts(anotherAccount)));
453-
});
454-
455-
it('should fail if addAllowedContract caller is not the owner', async function() {
456-
await utils.expectThrow(this.allowTokens.addAllowedContract(anotherAccount, { from: tokenDeployer }));
457-
});
458-
459-
it('should fail if removeAllowedContract caller is not the owner', async function() {
460-
await utils.expectThrow(this.allowTokens.removeAllowedContract(anotherAccount, { from: tokenDeployer }));
461-
});
462-
463-
it('should fail if addAllowedContract with zero address', async function() {
464-
await utils.expectThrow(this.allowTokens.addAllowedContract(utils.NULL_ADDRESS, { from: manager }));
465-
});
466-
467-
it('should fail if removeAllowedContract with zero address', async function() {
468-
await utils.expectThrow(this.allowTokens.removeAllowedContract(utils.NULL_ADDRESS, { from: manager }));
469-
});
470-
471-
});
472-
473440
describe('Ownable methods', async function() {
474441

475442
it('Should renounce ownership', async function() {

‎bridge/test/Bridge_test.js‎

Lines changed: 2 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -319,23 +319,17 @@ contract('Bridge', async function (accounts) {
319319
it('fail depositTo no wrapped currency set', async function () {
320320
const amount = web3.utils.toWei('1');
321321
const wrbtc = await WRBTC.new({ from: tokenOwner });
322-
const decimals = (await wrbtc.decimals()).toString();
323-
const symbol = await wrbtc.symbol();
324322

325323
await this.allowTokens.setToken(wrbtc.address, this.typeId, { from: bridgeManager });
326-
const originalBalance = await web3.eth.getBalance(tokenOwner);
327324
await utils.expectThrow(this.bridge.depositTo(anAccount, { from: tokenOwner, value: amount }));
328325
});
329326

330327
it('call depositTo from a contract', async function () {
331328
const amount = web3.utils.toWei('1');
332329
const wrbtc = await WRBTC.new({ from: tokenOwner });
333-
const decimals = (await wrbtc.decimals()).toString();
334-
const symbol = await wrbtc.symbol();
335330
const mockContract = await mockReceiveTokensCall.new(this.bridge.address)
336331
await this.bridge.setWrappedCurrency(wrbtc.address, { from: bridgeManager });
337332
await this.allowTokens.setToken(wrbtc.address, this.typeId, { from: bridgeManager });
338-
await this.allowTokens.addAllowedContract(mockContract.address, { from: bridgeManager });
339333
receipt = await mockContract.callDepositTo(anAccount, { from: tokenOwner, value: amount });
340334
utils.checkRcpt(receipt);
341335

@@ -345,17 +339,6 @@ contract('Bridge', async function (accounts) {
345339
assert.equal(isKnownToken, true);
346340
});
347341

348-
it('fail call depositTo from a contract if not allowed', async function () {
349-
const amount = web3.utils.toWei('1');
350-
const wrbtc = await WRBTC.new({ from: tokenOwner });
351-
const decimals = (await wrbtc.decimals()).toString();
352-
const symbol = await wrbtc.symbol();
353-
const mockContract = await mockReceiveTokensCall.new(this.bridge.address)
354-
await this.bridge.setWrappedCurrency(wrbtc.address, { from: bridgeManager });
355-
await this.allowTokens.setToken(wrbtc.address, this.typeId, { from: bridgeManager });
356-
await utils.expectThrow(mockContract.callDepositTo(anAccount, { from: tokenOwner, value: amount }));
357-
});
358-
359342
it('receiveTokens approve and transferFrom for ERC777', async function () {
360343
const amount = web3.utils.toWei('1000');
361344
const granularity = '1000';
@@ -467,13 +450,12 @@ contract('Bridge', async function (accounts) {
467450
assert.equal(isKnownToken, true);
468451
});
469452

470-
it('tokensReceived for ERC777 with whitelisted contract', async function () {
453+
it('tokensReceived for ERC777 called with contract', async function () {
471454
const amount = web3.utils.toWei('1000');
472455
const granularity = '100';
473456
const erc777 = await SideToken.new("ERC777", "777", tokenOwner, granularity, { from: tokenOwner });
474457
await this.allowTokens.setToken(erc777.address, this.typeId.toString(), { from: bridgeManager });
475458
const mockContract = await mockReceiveTokensCall.new(this.bridge.address);
476-
await this.allowTokens.addAllowedContract(mockContract.address, { from: bridgeManager });
477459
await erc777.mint(mockContract.address, amount, "0x", "0x", {from: tokenOwner });
478460
const originalTokenBalance = await erc777.balanceOf(mockContract.address);
479461
const userData = anAccount.toLowerCase();
@@ -826,21 +808,13 @@ contract('Bridge', async function (accounts) {
826808
await utils.expectThrow(this.bridge.receiveTokensTo(newToken.address, tokenOwner, amount, { from: tokenOwner }));
827809
});
828810

829-
it('receiveTokens should work calling from an allowed contract', async function () {
811+
it('receiveTokens should work calling from a contract', async function () {
830812
let otherContract = await mockReceiveTokensCall.new(this.bridge.address);
831-
this.allowTokens.addAllowedContract(otherContract.address, { from: bridgeManager });
832813
const amount = web3.utils.toWei('1000');
833814
await this.token.transfer(otherContract.address, amount, { from: tokenOwner });
834815
await otherContract.callReceiveTokens(this.token.address, tokenOwner, amount);
835816
});
836817

837-
it('receiveTokens should reject calling from a contract', async function () {
838-
let otherContract = await mockReceiveTokensCall.new(this.bridge.address);
839-
const amount = web3.utils.toWei('1000');
840-
await this.token.transfer(otherContract.address, amount, { from: tokenOwner });
841-
await utils.expectThrow(otherContract.callReceiveTokens(this.token.address, tokenOwner, amount));
842-
});
843-
844818
it('rejects to receive tokens greater than max tokens allowed 18 decimals', async function() {
845819
let limit = await this.allowTokens.typeLimits(this.typeId);
846820
let maxTokensAllowed = limit.max;

0 commit comments

Comments
 (0)