Several standards that can be used by a client to sign data using an Ethereum key:
-eth_sign:
-personal_sign: The method prefixed any signed data with "\x19Ethereum Signed Message:\n" which meant if one was to sign transaction data, the added prefix string would make it an invalid transaction.
-EIP-712: more complex use cases, this is done by signing hashes of all required configuration data (address, chain id, version, data types) + the actual data itself.
***Which signing standards should I use?'. From a contract's perspective use the latest EIP-712 standard! eth_sign is not safe and personal_sign is mostly useful for implementing user sign in features. In your contracts stick to EIP-712.

Now let's see how to implement EIP-712 in Solidity. The rough idea is
-compute a domain hash which captures the configuration data of contract address and chainId
-compute typed data hash
-combine both hashes and use it inside ecrecover

***Security issues with ecrecover + solution
1) In some cases ecrecover can return a random address instead of 0 for an invalid signature. This is prevented above by the owner address inside the typed data.
2) Signature are malleable, meaning you might be able to create a second also valid signature for the same data. In our case we are not using the signature data itself (which one may do as an id for example).
3) An attacker can construct a hash and signature that look valid if the hash is not computed within the contract itself.
4) In practice I would recommend once again to use the Openzeppelin contracts. Their ECDSA implementation solves all three problems and they have an EIP-712 implementation (still a draft but usable in my opinion). Not only is this easier to use, but they also have further improvements:
—caching mechanism for eip712DomainHash, so it's only calculated whenever chainId changes (so usually just once)
—additional security checks for the signature as mentioned above
—-ability to send signature as string


SIGNATURE REPLAY ATTACK:
1) Can be performed on the same contract (keeping track of what transactions have been executed inside the contract and nonces)
2) Same code, different address (Adding the address of the contract inside the signature, nounce)
3) A contract created with CREATE2 with self-destruct function inside (Deployed at the same address) Can't be prevented


https://solidity-by-example.org/hacks/signature-replay/

#EDUCATION

1) bytes32 typeHash = keccak256(
"EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"
);
bytes32 hashedName = keccak256(bytes(name));
bytes32 hashedVersion = keccak256(bytes(version));

2) domainSeparator = keccak256(abi.encode(typeHash, nameHash, versionHash, block.chainid, address(this)));

3) bytes32 private constant _PERMIT_TYPEHASH =
keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)");

4) bytes32 structHash = keccak256(abi.encode(_PERMIT_TYPEHASH, owner, spender, value, _useNonce(owner), deadline));

The necessary functionality
5) keccak256(abi.encodePacked("\x19\x01", domainSeparator, structHash)); <- How the signature is being constructed, the domainSeparator

Original code:
bytes32 digest = keccak256(abi.encodePacked("\x19\x01", keccak256(abi.encode(PERMIT_TYPEHASH, holder, spender, nonce, expiry, allowed))):

RESULT:
1) Digest has missing domainSeparator
2) The value was also missed -> value MaX (dangerous) or incorrect signature
3) pass digest to mapping()
4) block.timestamp check
5) get nonce inside of the function, delete the parameter nonce
#AUDIT #DONE

—Learn from Reading Audit Reports (Sturdy Report)
https://www.youtube.com/watch?v=9-Ye1HTvQIU

—My First Bounty Award
https://www.youtube.com/watch?v=vNB1t2Rpbtw

—Unhacked CTF - Reaper
https://www.youtube.com/watch?v=1xnMxdfYOG0

—Over 10k Achieved in Web3 Bounties
https://www.youtube.com/watch?v=P7omSW0S8nY

—My Study Methodology
https://www.youtube.com/watch?v=95Jj3yxVxNI

—My CV - Getting a JOB as a Smart Contract Auditor
https://www.youtube.com/watch?v=OFyB6BdTC8g

—My First Month as a Smart Contract Auditor
https://www.youtube.com/watch?v=AtTb_BpFZkM

—Code4rena Automation, Breaking Through Plateaus and Auditing Advice for Beginners/Intermediates
https://www.youtube.com/watch?v=gd5z2AKbvHk

—The ideal smart contract audit report
https://www.youtube.com/watch?v=cpyQw5mwE_0&t=331s

(code-423n4/2022-03-lifinance-findings)
Source , Code

Number: 1
Problem: approveERC20() uses unlimited approval
Severity: Medium

Impact:
approveERC20() is using unlimited approval.
Although unlimited approval is used by various DeFi platforms to minimize transaction fees and improve user experience, it introduces security risks as well.

Mitigation steps: approveERC20() -> approve( only required amount) (approve-spend pattern), or rather using increaseERC20Allowance()
/ decreaseERC20Allowance().
-to offer a choice: approve only what they need to spend at the time, or a much larger amount(future) (Zapper.fi, Curve.fi)
- EIP2612

Proof of Concept:
-Bancor, early 2020 (function calling tranferFrom was made public)
-Fucurombo (February 2021)
-Unicats (rugpull, 2020)
-Degen Money(frontend, mindless signing)
-Primitive Finance
-DeFi Safer

#CODE4ARENA #MEDIUM #REPORT #ERC20 #LIFINANCE

(code-423n4/2022-03-lifinance-findings)
Source, Code

Number: 2
Problem: approveMax for malicious routers leads to loss of funds.
AnyswapFacet can be exploited to approve arbitrary tokens.
Severity: Medium

Impact:
In AnyswapFacet.sol we parse arbitrary data in _anyswapData allowing an attacker to drain funds (ERC20 or native tokens) from the LiFi contract.

Functions effected:
-AnyswapFacet.startBridgeTokensViaAnyswap()
-AnyswapFacet.swapAndStartBridgeTokensViaAnyswap()
(multiple call of underlying() that can replace the the token at some point)

Proof of Concept
This attack works -in AnyswapFacet.startBridgeTokensViaAnyswap() by having a malicious _anyswapData.token which may change the value return in IAnyswapToken(_anyswapData.token).underlying() -> allows for transferring these malicious ERC20 tokens to pass the required balance checks.
-execute the following which will either approve or transfer funds to _anyswapData.router for a different underlyingtoken to the one which supplied the funds to LiFi.
-Since _anyswapData.router is an address in the attackers control they either are transferred native tokens or they have an allowance of ERC20 tokens that they can spend arbitrarily.

Mitigation steps:
Consider whitelisting both Anyswap tokens and Anyswap routers (using two distinct whitelists) restricting the attackers ability to use malicious contracts for this attack.
Consider also only calling IAnyswapToken(_anyswapData.token).underlying() once and passing this value to _startBridge().

There can be fund leftover in the contract under normal operation, for example this tx. In fact, ~$300 worth of token is left in the LI.Fi smart contract on ETH mainnet 0x5a9fd7c39a6c488e715437d7b1f3c823d5596ed1 as of block 14597316. I don't think this is High Risk because the max amount lost is no more than allowed slippage, which can be loss to MEV too.

#CODE4ARENA #MEDIUM #REPORT #ERC20 #LIFINANCE