godotengine/godot
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix multiplayer replication crash in on_sync_receive.

Browse Source 
A number of early continue cases applied the packet-provided `size`
without validation, allowing large uint32_t values to be treated as
negative offsets and leading to segfaults.

Now, we validate `size` against the buffer length immediately to avoid a
crash.

This could be triggered by receiving sync data for a synchronizer who's
root node had just been removed, since the code path that checked for
unusable sync state failed to advance the offset. Thus the next read
could interpret part of the payload as such an invalid `size`.

Now, we properly advance the read offset in that case (and raise a
better error).
This commit is contained in:
baptr 2023-02-14 02:40:06 -08:00
parent b7723a01d9
commit 2eadbe7b78
1 changed files with 4 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
modules/multiplayer/scene_replication_interface.cpp
View File

@ -742,6 +742,7 @@ Error SceneReplicationInterface::on_sync_receive(int p_from, const uint8_t *p_bu
ofs += 4;
uint32_t size = decode_uint32(&p_buffer[ofs]);
ofs += 4;
ERR_FAIL_COND_V(size > uint32_t(p_buffer_len - ofs), ERR_INVALID_DATA);
MultiplayerSynchronizer *sync = nullptr;
if (net_id & 0x80000000) {
sync = Object::cast_to<MultiplayerSynchronizer>(multiplayer->get_path_cache()->get_cached_object(p_from, net_id & 0x7FFFFFFF));
@ -756,14 +757,15 @@ Error SceneReplicationInterface::on_sync_receive(int p_from, const uint8_t *p_bu
}
Node *node = sync->get_root_node();
if (sync->get_multiplayer_authority() != p_from || !node) {
ERR_CONTINUE(true);
// Not valid for me.
ofs += size;
ERR_CONTINUE_MSG(true, "Ignoring sync data from non-authority or for missing node.");
}
if (!sync->update_inbound_sync_time(time)) {
// State is too old.
ofs += size;
continue;
}
ERR_FAIL_COND_V(size > uint32_t(p_buffer_len - ofs), ERR_BUG);
const List<NodePath> props = sync->get_replication_config()->get_sync_properties();
Vector<Variant> vars;
vars.resize(props.size());