Disable Form Tampering Protection for comment forms (fixes #1809)

[Yorwba]

CakePHP's FormHelper uses a SHA1 HMAC signature to protect hidden fields against modification by malicious users. The signature also covers the session_id, which means that if the session_id changes, all previously loaded forms break.

For the comment forms, the only hidden field is the sentence_id. Being able to modify it doesn't grant malicious users any interesting capabilities. If they want to post a comment on a different sentence, they can already do that.

---

I'm not sure whether this protection should be enabled for any form. There are a few other issues mentioning black-holed requests when submitting a form: #1922, #1955, #2198. I'd hope that in each case the form is validated on the backend and changing parameters on the frontend won't allow bypassing that even without the signature. But I guess it's nice to have in principle, as a kind of defense-in-depth?

[trang]

Just as a note, the form tampering projection also prevents a couple of other things aside from modification of hidden fields:

• Unknown fields cannot be added to the form.
• Fields cannot be removed from the form.

https://book.cakephp.org/3/en/controllers/components/security.html#form-tampering-prevention

I'm not sure whether this protection should be enabled for any form.
...
But I guess it's nice to have in principle, as a kind of defense-in-depth?

Yea, I also can't think of a reason to enable this protection on any form, except to have an extra line of defense in case the backend is not properly doing its validation job.

[Yorwba]

Just as a note, the form tampering projection also prevents a couple of other things aside from modification of hidden fields:

* Unknown fields cannot be added to the form.

* Fields cannot be removed from the form.

https://book.cakephp.org/3/en/controllers/components/security.html#form-tampering-prevention

Thanks for pointing this out. I guess this opens up potential security holes if unset parameters bypass the validation logic somehow, or if the received data is used generically, so that additional keys might end up e.g. in an SQL query. I've had a second look at the two endpoints affected here and I don't think they're vulnerable.

I'd considered disabling this protection globally, but now I think it's better to check each endpoint individually to see whether it is safe to unlock.