Please sign in to comment.
Fix randomly rejected form field submits (#8091)
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.
- Loading branch information