Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SecurityComponent::_validatePost() not working anymore for https-forms embedded in http pages #3442

Closed
casdevs opened this issue May 6, 2014 · 8 comments
Assignees
Labels
Milestone

Comments

@casdevs
Copy link

casdevs commented May 6, 2014

I'm having a problem with the latest SecurityComponent::_validatePost() changes now taking into account a form's url when calculating the security hash. Since cakephp 2.4.8, my login form/action is broken, leading to a blackhole error.

I'm embedding a login form with action attribute set to "https://hostname/login" in a http view to ensure the login credentials get posted over https.

From what I can see in FormHelper::create(), a full url containing protocol, hostname and action set in $options['url'] finds its way into $this->_lastAction without modifications.
However, in SecurityComponent::_validatePost(), the according url to be compared is read from $this->request->here() which doesn't contain protocol and hostname. This results in the hashes not being identical and the form submission being rejected.

Is it no longer possible to render a form with full https protocol + hostname + action within a http view, or shouldn't the protocol/hostname part always be removed before setting $this->_lastAction by FormHelper::create()?

Or is there another way I'm not aware of to ensure posting a form to https even if the form is rendered in a http page, without using full protocol/hostname its action attribute?

@dereuromark dereuromark added this to the 2.4.10 milestone May 6, 2014
@casdevs
Copy link
Author

casdevs commented May 6, 2014

just another thought: wouldn't it be even better to always include the full protocol/hostname when creating/comparing the security hash to ensure that no one can post forms accross different hosts executing the same cakephp web application?

@lorenzo lorenzo closed this as completed May 6, 2014
@lorenzo lorenzo reopened this May 6, 2014
@lorenzo
Copy link
Member

lorenzo commented May 6, 2014

Oops, fat fingers

@lorenzo
Copy link
Member

lorenzo commented May 6, 2014

I think your idea could work, are you able to produce a patch for this?

@markstory
Copy link
Member

@stefan-hausmann Let me know if you get stuck, I can work on this tonight. Either always using the absolute URL or ensuring only the path + query are used in both places is reasonable.

@markstory markstory self-assigned this May 6, 2014
@casdevs
Copy link
Author

casdevs commented May 6, 2014

@lorenzo, @markstory: I could produce a patch for this, but since I've never done that before, it would take a while (especially including the test cases) and I'm very busy this week. So if you think it is not that urgent, I could provide a patch on next weekend.

On the other hand, if Mark can handle this tonight, I'm happy too :) Either way, I think always using the absolute url would be the better choice because then reusing hashes on other servers with the same cake app (although probably being a minor attack vector) is prevented also.

Please give feedback how we should proceed.

Stefan

@casdevs
Copy link
Author

casdevs commented May 6, 2014

just for those of you who are currently suffering from the same problem, a quick fix is to simply comment out the lines

$this->request->here() (~ line 514 in SecurityComponent.php)
and
$this->_lastAction (~ line 576 in FormHelper.php)

so that the form's url is no longer used when computing the hashes until this gets fixed.

@markstory
Copy link
Member

I'll take care of this tonight. 😄

@casdevs
Copy link
Author

casdevs commented May 6, 2014

@markstory: great, thanks!

markstory added a commit that referenced this issue May 7, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants