OpenZeppelin / OpenZeppelin/openzeppelin-contracts
Consider renaming "to" in "safeTransfer" flows to remind about unsafe external call
Nobody has claimed this yet.
- Dominant language
- Solidity
- Stars
- 27.2k
- Forks
- 12.4k
- Avg merge
- 2d 19h
- Merged PRs (30d)
- 33
Description
An unsafe external call is made to innocent looking to.
https://www.paradigm.xyz/2021/08/the-dangers-of-surprising-code/
A suggestion that comes to mind is to use "untrustedTo" instead of "to", but any other name would be a good reminder about the dangers of untrusted external calls.
The rename should propagate back to the safeTransfer functions like https://github.com/OpenZeppelin/openzeppelin-contracts/blob/b174f067df18e3214f290c62d50f6721e87c05cf/contracts/token/ERC721/ERC721.sol#L168
Unsafe external calls is pretty much the first thing in the smart contract best practices, and marking untrusted contracts is the
first recommendation: https://consensys.github.io/smart-contract-best-practices/recommendations/#mark-untrusted-contracts
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the linked contracts/token/ERC721/ERC721.sol implementation, reading the safeTransfer functions and the external call that uses to. Trace the related safe-transfer flow, then rename the parameter consistently to signal that it may be untrusted; done means the affected interfaces and implementations use the new name without changing behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- solidity
- Domain
- blockchain, security
- Issue type
- Refactor
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100