Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebsocketServer crash on wrong rpc id #31106

Closed
Paquebot opened this issue Aug 5, 2019 · 1 comment · Fixed by #31482
Closed

WebsocketServer crash on wrong rpc id #31106

Paquebot opened this issue Aug 5, 2019 · 1 comment · Fixed by #31482
Assignees
Milestone

Comments

@Paquebot
Copy link

Paquebot commented Aug 5, 2019

Godot version:
3.1 and 834d07c (master)

OS/device including version:
Linux_x86_64

Issue description:
Websocket server crash on null deref when receiving wrong destination id.
It seems p_to parameter in https://github.com/godotengine/godot/blob/master/modules/websocket/websocket_multiplayer_peer.cpp#L272 is not sanitized and can lead to a null deref if it does not (longer?) exist.

Steps to reproduce:
Client A trying to send a RPC to a disconnected client B through rpc_unreliable_id or rpc__id. It depends on a race if client has up-to-date peers list or not. Seems to bug more from HTML5 client than native client.

Backtrace:

[1] /lib/x86_64-linux-gnu/libc.so.6(+0x37840) [0x7f44cf7fa840] (??:0)
[2] WebSocketMultiplayerPeer::_server_relay(int, int, unsigned char const*, unsigned int) (/home/pc/godot/modules/websocket/websocket_multiplayer_peer.cpp:272 (discriminator 2))
[3] WebSocketMultiplayerPeer::_process_multiplayer(Ref<WebSocketPeer>, unsigned int) (/home/pc/godot/modules/websocket/websocket_multiplayer_peer.cpp:305 (discriminator 2))
[4] WebSocketServer::_on_peer_packet(int) (/home/pc/godot/modules/websocket/websocket_server.cpp:73 (discriminator 2))
[5] wsl_msg_recv_callback(wslay_event_context*, wslay_event_on_msg_recv_arg const*, void*) (/home/pc/godot/modules/websocket/wsl_peer.cpp:155)
[6] /home/pc/godot/bin/godot.x11.tools.64(wslay_event_recv+0x93e) [0x196276b] (/home/pc/godot/thirdparty/wslay/wslay_event.c:753)
[7] WSLPeer::_wsl_poll(WSLPeer::PeerData*) (/home/pc/godot/modules/websocket/wsl_peer.cpp:78)
[8] WSLPeer::poll() (/home/pc/godot/modules/websocket/wsl_peer.cpp:232)
[9] WSLServer::poll() (/home/pc/godot/modules/websocket/wsl_server.cpp:178)
[10] MultiplayerAPI::poll() (/home/pc/godot/core/io/multiplayer_api.cpp:98)
[11] SceneTree::idle(float) (/home/pc/godot/scene/main/scene_tree.cpp:512)
[12] Main::iteration() (/home/pc/godot/main/main.cpp:1935)
[13] OS_X11::run() (/home/pc/godot/platform/x11/os_x11.cpp:3178)
[14] /home/pc/godot/bin/godot.x11.tools.64(main+0x104) [0x13915e6] (/home/pc/godot/platform/x11/godot_x11.cpp:57)
[15] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xeb) [0x7f44cf7e709b] (??:0)
[16] /home/pc/godot/bin/godot.x11.tools.64(_start+0x2a) [0x139142a] (??:?)

Proposed patch
Paquebot@4de9500

@Faless
Copy link
Collaborator

Faless commented Aug 19, 2019

@Paquebot thanks for spotting and reporting this.
Referenced PR should fix the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants