Contact Form Text Widget: Contact Form URL field is not referral URL

[jpswade]

Issue

The "Contact Form URL" field received in the email is incorrect. You would expect to receive the referral URL.

Steps to reproduce:

1. Insert a text widget into your widget area.
2. Visit a page (eg: http://blog.example.co.uk/about/)
3. Fill out and submit the form

   Actual:

Your Name: James Wade Your Email: jw@example.co.uk Enquiry Type: Test

Time: October 10, 2014 at 2:45 pm IP Address: 123.123.123.123 Contact Form URL: http://blog.example.co.uk/

Expected:

Your Name: James Wade Your Email: jw@example.co.uk Enquiry Type: Test

Time: October 10, 2014 at 2:45 pm IP Address: 123.123.123.123 Contact Form URL: http://blog.example.co.uk/about/

[jpswade]

The problematic line is here:

https://github.com/Automattic/jetpack/blob/master/modules/contact-form/grunion-contact-form.php#L1356

$url     = $widget ? home_url( '/' ) : get_permalink( $post->ID );

[georgestephanis]

Interesting. The problem being that if we blindly accepted the HTTP_REFERER attribute and passed that along through, it would need a good look at sanitizing. I'm not opposed to changing it, but we'd need some soak time to make sure we don't open up any security holes.

[jpswade]

You may prefer to use this instead:

$current_url = ( is_ssl() ? 'https://' : 'http://' ) . $_SERVER['HTTP_HOST'] . attribute_escape($_SERVER['REQUEST_URI']);

Then simply wrap that in the form as a hidden field:

<input type="hidden" name="ref" value="<?php echo $current_url; ?>" />

This is the safest way to achieve it and avoids potential security holes that you might see using the referer.

[georgestephanis]

@jpswade Please don't advocate for ternaries with is_ssl() -- just use core's set_url_scheme() function instead. Far more readable.

[jpswade]

Don't advocate? You mean like this?

Practice what you preach...

[jeherve]

@jpswade We updated most of the is_ssl calls a while ago, in #425, but if you find more calls that could be updated to use set_url_scheme(), do not hesitate to create a Pull Request!

[jpswade]

I appreciate that there are multiple ways to skin a cat. I went through a number of iterations before settling on this version as it mostly matched the existing code of this project.

If you follow the link I posted, you'll find a number of references to is_ssl (even in ternaries) which forms the basis for the workflow I matched. They aren't hard to find.

That issue aside, I understand that this means that you agree with the solution in principle, it's now just trivialities.

[georgestephanis]

It was a drive by comment, please don't assume that silence implies consent. I'll review the rest when I have a chance, I'm currently at the WordPress Community Summit and don't have time to audit anything in depth.

[jpswade]

@georgestephanis your "drive by comment" came across as trivial, elitist and most of all unhelpful.

I look forward to a more in depth audit and response in the new year.

All the best.

[stale[bot]]

This issue has been marked as stale. This happened because:

• It has been inactive in the past 6 months.
• It hasn’t been labeled `[Pri] Blocker`, `[Pri] High`.

No further action is needed. But it's worth checking if this ticket has clear reproduction steps and it is still reproducible. Feel free to close this issue if you think it's not valid anymore — if you do, please add a brief explanation.

[github-actions[bot]]

This issue has been marked as stale. This happened because:

• It has been inactive in the past 6 months.
• It hasn’t been labeled `[Pri] Blocker`, `[Pri] High`, etc.

No further action is needed. But it's worth checking if this ticket has clear reproduction steps and it is still reproducible. Feel free to close this issue if you think it's not valid anymore — if you do, please add a brief explanation.