Tickets :: [#14148] vacation, spam & forward double encoding

6.0.0-alpha12

6/12/25

Summary	vacation, spam & forward double encoding
Queue	Horde Framework Packages
Type	Bug
State	Resolved
Priority	1. Low
Owners	mrubinsk (at) horde (dot) org
Requester	dbgarcia (at) gmv (dot) com
Created	10/30/2015 (3513 days ago)
Due
Updated	11/03/2015 (3509 days ago)
Assigned	11/02/2015 (3510 days ago)
Resolved	11/03/2015 (3509 days ago)
Github Issue Link
Github Pull Request
Milestone
Patch	No

11/03/2015 02:23:04 PM	Michael Rubinsky	State ⇒ Resolved

11/03/2015 11:35:31 AM	dbgarcia (at) gmv (dot) com	Comment #19	Reply to this comment
Thanks! it works with the new files, Best regards

11/02/2015 07:53:43 PM	Git Commit	Comment #18	Reply to this comment
Changes have been made in Git (master): commit a8d1297249b89ad441f3c9c59256ba7de36c11ab Author: Michael J Rubinsky <mrubinsk@horde.org> Date: Mon Nov 2 14:51:13 2015 -0500 Use full URL, since these will be html encoded by Horde_Form. This still smells a tiny bit funny, since it requires knowing what Horde_Form does internally, but... Related to ~~Bug: 14148~~ ingo/lib/Basic/Forward.php \| 2 +- ingo/lib/Basic/Spam.php \| 2 +- ingo/lib/Basic/Vacation.php \| 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) http://github.com/horde/horde/commit/a8d1297249b89ad441f3c9c59256ba7de36c11ab

11/02/2015 07:53:12 PM	Git Commit	Comment #17	Reply to this comment
Changes have been made in Git (FRAMEWORK_5_2): commit d2ec7373f8dfa2c71784be4d9978e52e2228d857 Author: Michael J Rubinsky <mrubinsk@horde.org> Date: Mon Nov 2 14:51:13 2015 -0500 Use full URL, since these will be html encoded by Horde_Form. This still smells a tiny bit funny, since it requires knowing what Horde_Form does internally, but... Related to ~~Bug: 14148~~ ingo/lib/Basic/Forward.php \| 2 +- ingo/lib/Basic/Spam.php \| 2 +- ingo/lib/Basic/Vacation.php \| 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) http://github.com/horde/horde/commit/d2ec7373f8dfa2c71784be4d9978e52e2228d857

11/02/2015 07:49:36 PM	Michael Rubinsky	Comment #16	Reply to this comment
Ah. I get it now :) Still, this since these URLs will never have another parameter added in the action URL (as you said, they are POST forms), this still fixes the original issue. Still, for good measure, I'll force Horde::url to be a full URL since this will prevent the & encoding from Horde_Url.

11/02/2015 07:32:10 PM	Jan Schneider	Comment #15	Reply to this comment
I guess what we actually need to do is add the session id to the form, and not allow Horde:url to append it so we can still get a "clean" url. Nope, this doesn't change anything, beside that it may happen that the URL only contains a single parameter now, so there's no ampersand to be double encoded. But as soon as one of these gets another parameter, it's broken again. Adding the session id as a form field is still better though. Actually we shouldn't use parameters in the action URL at all, technically, since these are POST forms, not GET forms. There is no browser that would choke on this behaviour though.

11/02/2015 07:28:37 PM	Jan Schneider	Comment #14	Reply to this comment
This isn't correct and opens a security issue in Horde_Form. We should probably rather make sure that we don't pass the encoded URL to Horde_Form from Ingo. Probably need to set ->raw in the passed Horde_Url. But it's not already encoded. Horde_Url objects are always encoded by default if printed (or casted to string fwiw). What was removed was the "action" attribute being run through htmlspecialchars, which isn't really appropriate for encoding an actual URL. e.g.: htmlspecialchars('/some/page.php?foo=bar&bar=foo') does not result in a valid, working url. It does. Because this happens where the URL is printed. The result is of course not a valid URL, but a correctly encoded URL to be embedded into a HTML page.

11/02/2015 07:02:04 PM	Michael Rubinsky	Comment #13 State ⇒ Feedback	Reply to this comment
Better?

11/02/2015 07:00:32 PM	Git Commit	Comment #12	Reply to this comment
Changes have been made in Git (master): commit 303d4d6a9a2a17dc2893da737e039880a4d46df8 Author: Michael J Rubinsky <mrubinsk@horde.org> Date: Mon Nov 2 13:57:35 2015 -0500 ~~Bug: 14148~~ Correctly add session id to form if it's needed. ingo/lib/Basic/Forward.php \| 8 ++++++-- ingo/lib/Basic/Spam.php \| 8 ++++++-- ingo/lib/Basic/Vacation.php \| 8 ++++++-- 3 files changed, 18 insertions(+), 6 deletions(-) http://github.com/horde/horde/commit/303d4d6a9a2a17dc2893da737e039880a4d46df8

11/02/2015 07:00:11 PM	Git Commit	Comment #11	Reply to this comment
Changes have been made in Git (master): commit 3dde111136ba3ff170501acb5e9b9993debfcd5f Author: Michael J Rubinsky <mrubinsk@horde.org> Date: Mon Nov 2 13:59:17 2015 -0500 Revert "~~Bug: 14148~~ Don't convert html special entities in form action." This reverts commit 9cc5cb3a13289e2ced64133cf98db3eab2431bb7. framework/Form/lib/Horde/Form/Renderer.php \| 1 + 1 files changed, 1 insertions(+), 0 deletions(-) http://github.com/horde/horde/commit/3dde111136ba3ff170501acb5e9b9993debfcd5f

11/02/2015 06:58:17 PM	Git Commit	Comment #10	Reply to this comment
Changes have been made in Git (FRAMEWORK_5_2): commit 3309278cd40275b31ef18f058247f89a23b205d7 Author: Michael J Rubinsky <mrubinsk@horde.org> Date: Mon Nov 2 13:57:35 2015 -0500 ~~Bug: 14148~~ Correctly add session id to form if it's needed. ingo/lib/Basic/Forward.php \| 8 ++++++-- ingo/lib/Basic/Spam.php \| 8 ++++++-- ingo/lib/Basic/Vacation.php \| 8 ++++++-- 3 files changed, 18 insertions(+), 6 deletions(-) http://github.com/horde/horde/commit/3309278cd40275b31ef18f058247f89a23b205d7

11/02/2015 06:24:21 PM	Michael Rubinsky	Comment #9	Reply to this comment
I guess what we actually need to do is add the session id to the form, and not allow Horde:url to append it so we can still get a "clean" url.

11/02/2015 06:22:39 PM	Michael Rubinsky	Comment #8	Reply to this comment
This isn't correct and opens a security issue in Horde_Form. We should probably rather make sure that we don't pass the encoded URL to Horde_Form from Ingo. Probably need to set ->raw in the passed Horde_Url. But it's not already encoded. What was removed was the "action" attribute being run through htmlspecialchars, which isn't really appropriate for encoding an actual URL. e.g.: htmlspecialchars('/some/page.php?foo=bar&bar=foo') does not result in a valid, working url.

11/02/2015 05:34:28 PM	Jan Schneider	Comment #7	Reply to this comment
This isn't correct and opens a security issue in Horde_Form. We should probably rather make sure that we don't pass the encoded URL to Horde_Form from Ingo. Probably need to set ->raw in the passed Horde_Url.

11/02/2015 04:18:38 PM	Michael Rubinsky	State ⇒ Resolved

11/02/2015 04:18:23 PM	Michael Rubinsky	Comment #6 Version ⇒ Queue ⇒ Horde Framework Packages	Reply to this comment
Issue was in Horde_Form.

11/02/2015 04:17:44 PM	Git Commit	Comment #5	Reply to this comment
Changes have been made in Git (master): commit 9cc5cb3a13289e2ced64133cf98db3eab2431bb7 Author: Michael J Rubinsky <mrubinsk@horde.org> Date: Mon Nov 2 11:15:00 2015 -0500 ~~Bug: 14148~~ Don't convert html special entities in form action. framework/Form/lib/Horde/Form/Renderer.php \| 1 - 1 files changed, 0 insertions(+), 1 deletions(-) http://github.com/horde/horde/commit/9cc5cb3a13289e2ced64133cf98db3eab2431bb7

11/02/2015 03:59:18 PM	Michael Rubinsky	Comment #4 State ⇒ Assigned Assigned to Michael Rubinsky	Reply to this comment
Actually, can reproduce now. It only happens when you have cookies disabled.

11/02/2015 03:50:40 PM	Michael Rubinsky	Comment #3 Priority ⇒ 1. Low	Reply to this comment
Cannot reproduce. Please upgrade first, as your install is out of date.

10/30/2015 09:53:37 AM	dbgarcia (at) gmv (dot) com	Comment #2	Reply to this comment
Horde version is Groupware 5.2.5

10/30/2015 09:50:47 AM	dbgarcia (at) gmv (dot) com	Comment #1 Priority ⇒ 2. Medium State ⇒ Unconfirmed Patch ⇒ No Milestone ⇒ Summary ⇒ vacation, spam & forward double encoding Type ⇒ Bug Queue ⇒ Ingo	Reply to this comment
Hello, when we try to configure Ingo filters: vacation, forward or spam with buttons: "save", "save and enable" or "Return to rules list" it fails returning a "page not found" error. We have seen that URLs attached to those buttons seems to be double encoded (please note "&amp;" text): <!--a75c305b1c0a6022--><form action="/horde/ingo/basic.php?Horde=llmtcafuh0kmsuc8p7h8535rg0&amp;page=spam" method="post" name="ingo_form_spam" id="ingo_form_spam"> Could it be a double encoding issue? Bes regards.