If a formspec is submitted from a form fields handling
callback of another form (or "formspec shown from another
formspec"), the fields submitted for it can get
rejected by the form exploit mitigation subsystem with a
message like "'zorman2000' submitted formspec
('formspec_error:form2') but server hasn't sent formspec to
client, possible exploitation attempt" being sent to logs.
This was already reported as #7374 and a change was made
that fixed the simple testcase included with that bug
report but the bug still kept lurking around and popping
out in more complicated scenarios like the advtrains TSS
route programming UI.
Deep investigation of the problem revealed that this
sequence of events is entirely possible and leads to the
bug:
1. Server: show form1
2. Client *shows form1*
3. Client: submits form1
4. Server: show form2
5. Client: says form1 closed
6. Client *shows form2*
7. Client: submits form2
What happens inside the code is that when the server in
step 4 sends form2, the registry of opened forms is
updated to reflect the fact that form2 is now the valid
form for the client to submit. Then when in step 5 client
says "form1 was closed", the exploit mitigation subsystem
code deletes the registry entry for the client without
bothering to check whether the form client says was
closed just now is indeed the form that is recorded in
that entry as the valid form. Then later, in step 7 the
client tries to submit its valid form fields, these will
be rejected because the entry is missing.
It turns out the procedure where the broken code resides
already gets the form name so a simple "if" around the
offending piece of code fixes the whole thing. And
advtrains TSS agrees with that.
Reported by GCC
```
minetest/src/server.cpp:996:42: warning: ‘peer_id’ may be used uninitialized in this function [-Wmaybe-uninitialized]
errorstream << "ProcessData: peer=" << peer_id << e.what() << std::endl;
```
* Adds Lua methods 'set_rotation()' and 'get_rotation'. Also changed some method names to be more clear. Instead of an f32 being sent over network for yaw, now a v3f is sent for rotation on xyz axes. Perserved Lua method set_yaw/setyaw so that old mods still work, other wise to set yaw they would need to switch to set_rotation(0, yaw, 0).
* src/sky.cpp 146 warn V519 The 'suncolor_f.r' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 142, 146.
* src/sky.cpp 147 warn V519 The 'suncolor_f.g' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 143, 147.
* src/sky.cpp 148 warn V519 The 'suncolor_f.b' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 144, 148.
* src/threading/thread.cpp 63 err V730 Not all members of a class are initialized inside the constructor. Consider inspecting: m_thread_obj.
* src/server.cpp 3243 err V595 The 'log' pointer was utilized before it was verified against nullptr. Check lines: 3243, 3258.
* Drop EventManager from GameDef & do some client cleanups
* EventManager is only used by Client. Don't expose it on Server & GameDef for nothing
* Drop Client::event() in favor of direct calls to getEventManager
* Cleanup some event put from new + put to put(new)
* MtEvent: add Type(u8) enum
* This will enhance event performance & ensure stricter type
* Drop MtEvent::checkIs (unused)
* clang-tidy reported fixes
* Code style
* Move event_manager.h to the client directory as it's only used by client
Add EventManager unittests + switch to unordered_map as order is not important here
Drop a unused function
* Server: delegate mod management & config to ServerModConfiguration (rename it to ServerModManager)
* Use c++11 range based loops
* Add unittests + experimental/default mod as a test case to permit testing mod loading in future tests
* ObjectRef::set_local_animation: fix wrong lua return (should push a boolean, currently returns nil)
* ObjectRef::set_eye_offset: fix wrong lua return (should push a boolean, currently returns nil)
* Fix various Server functions which depends on RemotePlayer objet and return true/false when player object is nil whereas it's a caller implementation error. Change those bool functions to void and add sanitize_check call instead. Current callers are always checking player object validity
* Optimize Server::setClouds : use CloudParams object ref instead of attribute deserialization from structure & perform RemotePlayer::setCloudParams directly in server class like many other calls
* Optimize Server::SendCloudParams: use CloudParams object ref instead of deserialized attributes
This doesn't check the fields in anyway whatsoever so it should only be seen as a way to mitigate exploits, a last line of defense to make it harder to exploit bugs in mods, not as a reason to not do all the usually checks.
* Rename IWritableNodeDefManager to NodeDefManager
* Make INodeDefManager functions const
* Use "const *NodeDefManager" instead of "*INodeDefManager"
* Remove unused INodeDefManager class
* Merge NodeDefManager and CNodeDefManager
* Document NodeDefManager
Solves some issues with ModStorage functionality in mapgen threads
that occurred when mapgen threads continued to run after the main
server thread had stopped. Also shuts down mapgen threads before
shutdown callbacks are called.
* Make Player::peer_id server-side only and add getters and setters
Player::peer_id has no sense client side, move it to server, make it private and add setter and getter
Also add some PEER_ID_INEXISTENT instead of harcoded 0
* Add session_t typedef + remove unused functions
u16 peer_id is used everywhere, to be more consistent and permit some evolutions on this type in the future (i'm working on a PoC), uniformize u16 peer_id to SessionId peer_id
Implement network communication for channels
* Implement ModChannel manager server side to route incoming messages from clients to other clients
* Add signal handler switch on client & ModChannelMgr on client to handle channels
* Add Lua API bindings + client packet sending + unittests
* Implement server message sending
* Add callback from received message handler to Lua API using registration method
