0

The script I have works perfectly fine but I have done some reading and now I am wondering if this code is secure and if not what would be the secure & correct way of implementing it:

<?php
function get_current_page_url()

    {
        $url = $_SERVER['REQUEST_URI'];
        $main_url = explode('?', $url);
        $main_url = explode('/', $main_url[0]);
        return $main_url = end($main_url);
    }
?>

<script type="text/javascript">
var pageUrl = "<?php echo get_current_page_url() ?>?" + form_data;
</script>
5
  • 3
    why would the url of the current page be secret information? the user can see it all the time in the address bar. Commented Jan 2, 2016 at 17:37
  • 3
    also, why not just get that information from javascript directly? from the location object? Commented Jan 2, 2016 at 17:37
  • Or use relative Path.. Commented Jan 2, 2016 at 17:39
  • Agreed reading location.search directly from JS would seem to be a simpler approach. But yes, in any case, you have JavaScript injection and HTML injection here because you are not escaping for the surrounding context in your <?php echo. Commented Jan 3, 2016 at 14:08
  • @toskv because we are using mod_rewrite in a unique way and thus can not get it from JS directly Commented Jan 13, 2016 at 12:08

1 Answer 1

1

Can you write an answer to this question with the secure & correct way to implement it, preventing these security issues?

Injecting content from PHP into a JavaScript literal in a <script> block:

var current = <?php echo json_encode(get_current_page_url(), JSON_HEX_TAG); ?>;
var pageUrl = current + '?' + form_data;

JSON-encoding produces a string literal including the quotes. This is good enough for JavaScript because JSON literals are (nearly) a subset of JavaScript literals.

(The ‘nearly’ is because due to an oversight in design, raw characters U+2018 and U+2019 are allowable in JSON string literals but not JavaScript. This doesn't matter here as PHP encodes those and other non-ASCII characters to \u escapes anyway.)

The JavaScript is enclosed in a <script> block in an HTML document, so it's essential that if the string </script> is inside the PHP string it doesn't come out directly in the source—if it did it would end not just the string literal, but the whole script block. JSON_HEX_TAG prevents this happening by replacing the < with the escape \u003C. It is not wholly necessary from PHP 5.4 onwards as these versions also escape the / by default.

Using JSON_HEX_QUOT|JSON_HEX_TAG|JSON_HEX_AMP|JSON_HEX_APOS will encode more characters that are special in an HTML context, which isn't wholly necessary here but would allow you to inject into a string in an HTML <script> block as well as an XHTML <script> block or and HTML onxxx="..." event handler without having to perform the extra layer of htmlspecialchars() that could be necessary in these places.

Having said that, you are better off not injecting into JavaScript at all, because keeping track of multiple nested injection contexts is hard. It's better to keep JS out of your HTML documents entirely, ideally. You can inject the content into data- attributes with plain old HTML-escaping, like you should be using everywhere you put content into HTML, and then read the strings direct from the DOM:

<body data-current-url="<?php echo htmlspecialchars(get_current_page_url()); ?>">

...

var current = document.body.getAttribute('data-current-url');
var pageUrl = current + '?' + form_data;

Having said that, all of this is likely superfluous in this case because you can already read the document's URL directly from JavaScript using the location object. Unless you are doing something wacky involving mod_rewrite, it would be the same to say just:

var pageUrl = location.pathname.split('/').pop() + '?' + form_data;

and if all you want is to be able to make a relative link to the current page with a different query string, you don't even need to do that, you can simply use '?' + form_data.

Sign up to request clarification or add additional context in comments.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.