HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Navigation script and template

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
scriptnavigationtemplateand

Problem

Overview:

I'm using the following page template and scripts for navigation. I use a single page that loads menus and context content, and then use ajax or GET variable to load main-div's content, by passing $p variable to a php switch. Browser history is handled whith pushState function.

Main page:


  
  
  
     

    

          
    // Define state on first load
    function defineState() {
            "use strict";
            window.history.replaceState("", "", "");
        }   
    window.onload = defineState;

    function navegacion(dashboard, history) {

       "use strict";
       var urlPath = dashboard;
       var xmlhttp;
       var ActiveXObject;

       if (window.XMLHttpRequest) {
           // code for IE7+, Firefox, Chrome, Opera, Safari
           xmlhttp = new XMLHttpRequest();
       } else {
           // code for IE6, IE5
           xmlhttp = new ActiveXObject("Microsoft.XMLHTTP");
       }
       xmlhttp.onreadystatechange = function() {
           if (xmlhttp.readyState === 4 && xmlhttp.status === 200) {

                window.scrollTo(0, 0);

                //document.title = "TITLE";

                document.getElementById("main-div").innerHTML = xmlhttp.responseText;

                if(history !== 1) {
                    window.history.pushState(dashboard, "", "index.php?p="+urlPath);
                }

           }
       };
       xmlhttp.open("POST","assets/ajaxapi.php", true);
       xmlhttp.setRequestHeader("Content-type","application/x-www-form-urlencoded");
       xmlhttp.send("dashboard="+dashboard);

    }

    // Allows for back and forward browswe buttons

    window.onpopstate = function(e){
    "use strict";
    if(e.state){
        navegacion(e.state, 1);
    }
    };  

    

  


ajaxapi.php:

<?php
if (isset($_POST['dashboard'])) {
    $p = $_POST['dashboard'];

} else {
    $p = NULL;
}

require_once("./switch.php");


Could there be any performance or error issues with this code, or how could this

Solution

Security: XSS

You are open to reflected XSS via this payload:

p=foo","","");alert(1)//


Note that the injection takes place into a JavaScript context, which is worse than injections into HTML context because browser filters have a hard time catching it (if you try the payload with eg chrome, it will execute, while "alert(1) would not).

Standard HTML-encoding would solve your problem, but OWASP recommends escaping to prevent XSS in a JS context.

Misc

  • p isn't a great variable name. If it stands for page, just use $page instead (also for the GET index).



  • As your other variable and function names are in english, navegacion should be as well.

Code Snippets

p=foo","","");alert(1)//

Context

StackExchange Code Review Q#121203, answer score: 6

Revisions (0)

No revisions yet.