godotengine / godotengine/godot

`.rpc_id(` should not require `"call_local"` to run locally.

Open
#98,588 9 comments 0 reactions 0 assignees View on GitHub
discussion enhancement topic:multiplayer
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.