6.0.0-alpha12
6/12/25

[#14148] vacation, spam & forward double encoding
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

History
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;amp;" text):

<!--a75c305b1c0a6022--><form 
action="/horde/ingo/basic.php?Horde=llmtcafuh0kmsuc8p7h8535rg0&amp;amp;page=spam" method="post" name="ingo_form_spam" 
id="ingo_form_spam">

Could it be a double encoding issue?

Bes regards.

Saved Queries