godotengine / godotengine/godot
`.rpc_id(` should not require `"call_local"` to run locally.
- Dominant language
- C++
- Stars
- 117k
- Forks
- 26.8k
- PR merge metrics
- PR metrics pending
Description
### Tested versions
Reproduceable in all Godot 4.x version, including current `master` branch
### System information
All
### Issue description
Godot should trust the developer when calling `rpc_id(` on a locally owned node and continue the call locally even when `"call_local"` isn't part of it's `@rpc(`. There is clear security problems with trusting external calls. These security issues do not exist in local->local calls and adds confusion. If a developer is calling a function to a particular to a particular id, it should trust that they intended that. By blocking programmer intention to call on a target that just happens to be local owner, it adds confusion.
This diff should fix this and similify some of the `SceneRPCInterface::rpcp`, I can commit pull request this if this behavior change is agreeable.
```diff
diff --git a/modules/multiplayer/scene_rpc_interface.cpp b/modules/multiplayer/scene_rpc_interface.cpp
index 0938d7ef99..dc38586a37 100644
--- a/modules/multiplayer/scene_rpc_interface.cpp
+++ b/modules/multiplayer/scene_rpc_interface.cpp
@@ -477,57 +477,53 @@ Error SceneRPCInterface::rpcp(Object *p_obj, int p_peer_id, const StringName &p_
ERR_FAIL_COND_V_MSG(peer->get_connection_status() != MultiplayerPeer::CONNECTION_CONNECTED, ERR_CONNECTION_ERROR, "Trying to call an RPC via a multiplayer peer which is not connected.");
int caller_id = multiplayer->get_unique_id();
- bool call_local_native = false;
- bool call_local_script = false;
const RPCConfigCache &config_cache = _get_node_config(node);
uint16_t rpc_id = config_cache.ids.has(p_method) ? config_cache.ids[p_method] : UINT16_MAX;
ERR_FAIL_COND_V_MSG(rpc_id == UINT16_MAX, ERR_INVALID_PARAMETER,
vformat("Unable to get the RPC configuration for the function \"%s\" at path: \"%s\". This happens when the method is missing or not marked for RPCs in the local script.", p_method, node->get_path()));
const RPCConfig &config = config_cache.configs[rpc_id];
- ERR_FAIL_COND_V_MSG(p_peer_id == caller_id && !config.call_local, ERR_INVALID_PARAMETER, "RPC '" + p_method + "' on yourself is not allowed by selected mode.");
-
- if (p_peer_id == 0 || p_peer_id == caller_id || (p_peer_id < 0 && p_peer_id != -caller_id)) {
- if (rpc_id & (1 << 15)) {
- call_local_native = config.call_local;
- } else {
- call_local_script = config.call_local;
- }
- }
+ // ERR_FAIL_COND_V_MSG(p_peer_id == caller_id && !config.call_local, ERR_INVALID_PARAMETER, "RPC '" + p_method + "' on yourself is not allowed by selected mode.");
+ WARN_PRINT("hit this line");
if (p_peer_id != caller_id) {
_send_rpc(node, p_peer_id, rpc_id, config, p_method, p_arg, p_argcount);
}
- if (call_local_native) {
- Callable::CallError ce;
-
- multiplayer->set_remote_sender_override(multiplayer->get_unique_id());
- node->callp(p_method, p_arg, p_argcount, ce);
- multiplayer->set_remote_sender_override(0);
-
- if (ce.error != Callable::CallError::CALL_OK) {
- String error = Variant::get_call_error_text(node, p_method, p_arg, p_argcount, ce);
- error = "rpc() aborted in local call: - " + error + ".";
- ERR_PRINT(error);
- return FAILED;
- }
- }
-
- if (call_local_script) {
- Callable::CallError ce;
- ce.error = Callable::CallError::CALL_OK;
-
- multiplayer->set_remote_sender_override(multiplayer->get_unique_id());
- node->get_script_instance()->callp(p_method, p_arg, p_argcount, ce);
- multiplayer->set_remote_sender_override(0);
-
- if (ce.error != Callable::CallError::CALL_OK) {
- String error = Variant::get_call_error_text(node, p_method, p_arg, p_argcount, ce);
- error = "rpc() aborted in script local call: - " + error + ".";
- ERR_PRINT(error);
- return FAILED;
+ // call local:
+ // - if directly called on this peer
+ // - if broadcast to all peers
+ // - if broadcast to all peers except not this peer
+ if (p_peer_id == caller_id || ((p_peer_id == 0 && config.call_local) || (p_peer_id < 0 && p_peer_id != -caller_id))) {
+ if (rpc_id & (1 << 15)) { //call_local_native
+ Callable::CallError ce;
+
+ multiplayer->set_remote_sender_override(multiplayer->get_unique_id());
+ node->callp(p_method, p_arg, p_argcount, ce);
+ multiplayer->set_remote_sender_override(0);
+
+ if (ce.error != Callable::CallError::CALL_OK) {
+ String error = Variant::get_call_error_text(node, p_method, p_arg, p_argcount, ce);
+ error = "rpc() aborted in local call: - " + error + ".";
+ ERR_PRINT(error);
+ return FAILED;
+ }
+ } else { //call_local_script
+ Callable::CallError ce;
+ ce.error = Callable::CallError::CALL_OK;
+
+ multiplayer->set_remote_sender_override(multiplayer->get_unique_id());
+ node->get_script_instance()->callp(p_method, p_arg, p_argcount, ce);
+ multiplayer->set_remote_sender_override(0);
+
+ if (ce.error != Callable::CallError::CALL_OK) {
+ String error = Variant::get_call_error_text(node, p_method, p_arg, p_argcount, ce);
+ error = "rpc() aborted in script local call: - " + error + ".";
+ ERR_PRINT(error);
+ return FAILED;
+ }
}
}
+
return OK;
}
```
### Steps to reproduce
Calling any rpc_id with the id being the local player causes the rpc to block.
```gd
extends Node
func _ready() -> void:
call_remote.rpc_id(multiplayer.get_unique_id(),"This is being blocked, but shouldn't be.")
@rpc("any_peer","call_remote")
func call_remote(text):
print(text)
```
produces the following error:
```
ERROR: RPC 'echo' on yourself is not allowed by selected mode.
at: (modules\multiplayer\scene_rpc_interface.cpp:488)
```
### Minimal reproduction project (MRP)
```gd
extends Node
func _ready() -> void:
call_local.rpc("This should always run locally")
call_remote.rpc("This should never run locally")
call_local.rpc_id(multiplayer.get_unique_id(),"This should run regardless")
call_remote.rpc_id(multiplayer.get_unique_id(),"This is being blocked, but shouldn't be.")
@rpc("any_peer","call_local")
func call_local(text):
print(text)
@rpc("any_peer","call_remote")
func call_remote(text):
print(text)
```
In any project should be enough to repro this issue.
Contributor guide
Research direction
Start in modules/multiplayer/scene_rpc_interface.cpp, especially SceneRPCInterface::rpcp, and run the provided GDScript reproduction on Godot 4.x. Compare rpc_id(multiplayer.get_unique_id(), ...) with the call_local and call_remote configurations, while reviewing the existing RPC behavior and security concerns. Done means locally owned rpc_id calls behave as intended without weakening external-call security.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- networking
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 35/100