6.0.0-git
2019-03-21

[#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 2015-10-30 (1238 days ago)
Due
Updated 2015-11-03 (1234 days ago)
Assigned 2015-11-02 (1235 days ago)
Resolved 2015-11-03 (1234 days ago)
Milestone
Patch No

History
2015-11-03 14:23:04 Michael Rubinsky State ⇒ Resolved
 
2015-11-03 11:35:31 dbgarcia (at) gmv (dot) com Comment #19 Reply to this comment
Thanks! it works with the new files,

Best regards

2015-11-02 19:53:43 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
2015-11-02 19:53:12 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
2015-11-02 19:49:36 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.
2015-11-02 19:32:10 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.
2015-11-02 19:28:37 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.
2015-11-02 19:02:04 Michael Rubinsky Comment #13
State ⇒ Feedback
Reply to this comment
Better?
2015-11-02 19:00:32 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
2015-11-02 19:00:11 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
2015-11-02 18:58:17 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
2015-11-02 18:24:21 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.
2015-11-02 18:22:39 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.
2015-11-02 17:34:28 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.
2015-11-02 16:18:38 Michael Rubinsky State ⇒ Resolved
 
2015-11-02 16:18:23 Michael Rubinsky Comment #6
Version ⇒
Queue ⇒ Horde Framework Packages
Reply to this comment
Issue was in Horde_Form.
2015-11-02 16:17:44 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
2015-11-02 15:59:18 Michael Rubinsky Comment #4
Assigned to Michael Rubinsky
State ⇒ Assigned
Reply to this comment
Actually, can reproduce now.

It only happens when you have cookies disabled.


2015-11-02 15:50:40 Michael Rubinsky Comment #3
Priority ⇒ 1. Low
Reply to this comment
Cannot reproduce.

Please upgrade first, as your install is out of date.
2015-10-30 09:53:37 dbgarcia (at) gmv (dot) com Comment #2 Reply to this comment
Horde version is Groupware 5.2.5
2015-10-30 09:50:47 dbgarcia (at) gmv (dot) com Comment #1
Type ⇒ Bug
State ⇒ Unconfirmed
Priority ⇒ 2. Medium
Summary ⇒ vacation, spam & forward double encoding
Queue ⇒ Ingo
Milestone ⇒
Patch ⇒ No
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):

<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