docs(world): add comments for prohibitDirectCallback modifier #2312
Conversation
|
yonadaa
force-pushed
the
yonadaaa/explain-prohibit-direct-callback
branch
from
26d54da to
ae5d2c3
Compare
packages/world/src/World.sol
Outdated
| @@ -51,6 +51,9 @@ contract World is StoreData, IWorldKernel { | |||
|
|
|||
| /** | |||
| * @dev Prevents the World contract from calling itself. | |||
| * The world is not able to call itself; all operations to internal tables happen as internal library calls, and all calls to root system happen as a delegatecall to the system. | |||
There was a problem hiding this comment.
| * The world is not able to call itself; all operations to internal tables happen as internal library calls, and all calls to root system happen as a delegatecall to the system. | |
| * The `World` is not able to call itself; all operations to internal tables happen as internal library calls, and all calls to root systems happen as a `delegatecall` to the system. |
packages/world/src/World.sol
Outdated
| @@ -51,6 +51,9 @@ contract World is StoreData, IWorldKernel { | |||
|
|
|||
| /** | |||
| * @dev Prevents the World contract from calling itself. | |||
| * The world is not able to call itself; all operations to internal tables happen as internal library calls, and all calls to root system happen as a delegatecall to the system. | |||
| * However, since this is an important invariant, we make it explicit by reverting if `msg.sender` is `address(this)` in all `World` methods. | |||
| * If it was possible to make the `World` call itself, it would be possible to access internal tables that only the `World` should have access to. | |||
There was a problem hiding this comment.
this feels a bit incomplete, like we should either expand on how/why or just remove
If the
Worldis able to call itself viadelegatecallfrom a system, this would be a backdoor where that system would have "root" access to things like internal tables.
(maybe could workshop this exact sentence a bit more, but just providing a bit more context)
| _Prevents the World contract from calling itself. | ||
| If the World is able to call itself via delegatecall from a system, the system would have root access to context like internal tables, causing a potential vulnerability. | ||
| Ideally this should not happen because all operations to internal tables happen as internal library calls, and all calls to root systems happen as a `delegatecall` to the system. | ||
| However, since this is an important invariant, we make it explicit by reverting if `msg.sender` is `address(this)` in all `World` methods._ |
Closes #2106 by explaining why we use the
prohibitDirectCallbackmodifier.The comment is a rewording of the PR description from the original PR that added it #1563