Skip to content

Commit

Permalink
security audit suggestion
Browse files Browse the repository at this point in the history
  • Loading branch information
estebanmino committed Mar 20, 2019
1 parent 8610448 commit f7639d6
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 14 deletions.
26 changes: 16 additions & 10 deletions app/util/transactions.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,24 +140,30 @@ export function getMethodData(data) {
* Returns wether the given address is a contract
*
* @param {string} address - Ethereum address
* @param {string} transactionHash? - Transaction hash
* @returns {boolean} - Wether the given address is a contract
*/
export async function isSmartContractAddress(address) {
const cache = SmartContractAddresses.cache[address];
if (cache) {
return Promise.resolve(cache);
}

export async function isSmartContractAddress(address, transactionHash) {
address = toChecksumAddress(address);
// If in contract map we don't need to cache it
if (contractMap[address]) {
SmartContractAddresses.cache[address] = true;
return Promise.resolve(true);
}

// Look in cache memory only for specific time (transaction hash)
if (transactionHash) {
const cache = SmartContractAddresses.cache[(address, transactionHash)];
if (cache) {
return Promise.resolve(cache);
}
}
const { TransactionController } = Engine.context;
const code = address ? await TransactionController.query('getCode', [address]) : undefined;
// Geth will return '0x', and ganache-core v2.2.1 will return '0x0'
const codeIsEmpty = !code || code === '0x' || code === '0x0';
SmartContractAddresses.cache[address] = !codeIsEmpty;
// Save in cache for specific time (transaction hash)
if (transactionHash) {
SmartContractAddresses.cache[(address, transactionHash)] = !codeIsEmpty;
}
return !codeIsEmpty;
}

Expand Down Expand Up @@ -219,7 +225,7 @@ export async function getTransactionActionKey(transaction) {
return ret;
}
}
const toSmartContract = await isSmartContractAddress(to);
const toSmartContract = await isSmartContractAddress(to, transactionHash);
if (toSmartContract) {
// There is no data or unknown method data, if is smart contract
ret = SMART_CONTRACT_INTERACTION_ACTION_KEY;
Expand Down
36 changes: 32 additions & 4 deletions app/util/transactions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,28 @@ describe('Transactions utils :: isSmartContractAddress', () => {
await isSmartContractAddress('0x89d24A6b4CcB1B6fAA2625fE562bDD9a23260359');
expect(stub).not.toBeCalled();
});

it('isSmartContractAddress should call query if not cached', async () => {
Engine.context = MOCK_ENGINE.context;
const stub = spyOn(Engine.context.TransactionController, 'query');
await isSmartContractAddress('0x1', '0x123');
expect(stub).toBeCalled();
});

it('isSmartContractAddress should not call query if cached', async () => {
Engine.context = MOCK_ENGINE.context;
await isSmartContractAddress('0x1', '0x123');
const stub = spyOn(Engine.context.TransactionController, 'query');
await isSmartContractAddress('0x1', '0x123');
expect(stub).not.toBeCalled();
});

it('isSmartContractAddress should call query if only address was provided', async () => {
Engine.context = MOCK_ENGINE.context;
const stub = spyOn(Engine.context.TransactionController, 'query');
await isSmartContractAddress('0x1');
expect(stub).toBeCalled();
});
});

describe('Transactions utils :: getTransactionActionKey', () => {
Expand All @@ -109,21 +131,27 @@ describe('Transactions utils :: getTransactionActionKey', () => {
const randomData = '0x987654321';
const transferFromData = '0x23b872dd0000000000000000000000000000';
it('getTransactionActionKey send ether', async () => {
const get = await getTransactionActionKey({ transactionHash: '0x1', transaction: {} });
const get = await getTransactionActionKey({ transactionHash: '0x1', transaction: { to: '0x0' } });
expect(get).toEqual(SEND_ETHER_ACTION_KEY);
});

it('getTransactionActionKey send ether with empty data', async () => {
const get = await getTransactionActionKey({ transactionHash: '0x1', transaction: { data: '0x' } });
const get = await getTransactionActionKey({ transactionHash: '0x1', transaction: { data: '0x', to: '0x0' } });
expect(get).toEqual(SEND_ETHER_ACTION_KEY);
});
it('getTransactionActionKey send token', async () => {
const get = await getTransactionActionKey({ transactionHash: '0x2', transaction: { data: transferData } });
const get = await getTransactionActionKey({
transactionHash: '0x2',
transaction: { data: transferData, to: '0x0' }
});
expect(get).toEqual(SEND_TOKEN_ACTION_KEY);
});

it('getTransactionActionKey send collectible', async () => {
const get = await getTransactionActionKey({ transactionHash: '0x6', transaction: { data: transferFromData } });
const get = await getTransactionActionKey({
transactionHash: '0x6',
transaction: { data: transferFromData, to: '0x0' }
});
expect(get).toEqual(TRANSFER_FROM_ACTION_KEY);
});

Expand Down

0 comments on commit f7639d6

Please sign in to comment.