1

In a project I have to do for school it is required to prevent XSS (Cross-Site Scripting) attacks at my website, so tags such as <script type="text/javascript">alert("hello");</script> (which I can insert in every input tag in my website) shouldn't work (I should not see the alert window in this case). At school I was advised to use the php function htmlspecialchars, but the browser still shows me the alert window, when I insert this tag in a comment in my website. I can't understand what I'm doing wrong. (NOTE: I've also inserted the js code, but I think that the problem is in php, all the code works correctly)

$("#new-comment").on('click',function(){ 
  var $newReview = $("<input class='new-input' type='text' id='insert' name='insert' placeholder='write here...'>");  
  $("#reviews").append($newReview);   
  $newReview.on('keypress',function(e){
    if(e.which == 13){
      var comm = $(this);
      var datatime = new Date($.now());
      correctHour = printMysqlFormat(datatime);
      $.ajax({
        url:'reviews/reviews_query.php',
        data: {put:true, title: $("#right_title").text(), script: comm.val(), time: correctHour},     
        datatype:'json',
        success: function(json){
          var output = jQuery.parseJSON(json);
          var newName = "<span class='rev_name'>"+ output + "</span>";
          var newComment = "<span class='rev_comment'>"+ comm.val() + "<div class='rev_time'>" + correctHour + "</div></span>";
          $("#reviews > ul").html($("#reviews > ul").html()+"<li style='margin-bottom:20px'>" + newName + " " + newComment + "</li>");
        },
        error: function(e){
          console.log(e.message);
        }
      });
      $newReview.remove(); 
    }
  });
});

    if(isset($_GET["show"]) && isset($_GET["title"])){  //to show all comments
        $db = new PDO("mysql:host=localhost;dbname=music", "username", "password");
        $titolo = $_GET["title"];
        $ti = $db->quote($titolo);
        $rows = $db->query("SELECT * FROM reviews WHERE titolo=$ti ORDER BY ora ASC");
        if($rows->rowCount() == 0){ 
            echo 0;
        }else{
            $res = $rows->fetchALL(PDO::FETCH_ASSOC);
            echo json_encode($res);
        } 
    }
    
    if(isset($_GET["put"]) && isset($_GET["title"]) && isset($_GET["script"]) && isset($_GET["time"])){  //to write a new comment
        $db = new PDO("mysql:host=localhost;dbname=music", "username", "password");
        $username = $_SESSION["name"];
        $us = $db->quote($username);
        $titolo = $_GET["title"];
        $ti = $db->quote($titolo);
        $commento = $_GET["script"];
        $commento = htmlspecialchars($commento);
        $comm = $db->quote($commento);
        $timestamp = $_GET["time"];
        $tim = $db->quote($timestamp);
        $rows = $db->query("INSERT into reviews VALUES ($us, $ti, $comm, $tim)");     
        $res = $_SESSION["name"];
        echo json_encode($res); 
    } 

EDIT: I've tried with this code but nothing changes:

    if(isset($_GET["show"]) && isset($_GET["title"])){  
        $db = new PDO("mysql:host=localhost;dbname=music", "username", "password");
        $titolo = $_GET["title"];
        $ti = $db->quote($titolo);
        $rows = $db->query("SELECT * FROM reviews WHERE titolo=$ti ORDER BY ora ASC");
        if($rows->rowCount() == 0){
            echo 0;
        }else{
            $res = $rows->fetchALL(PDO::FETCH_ASSOC);
            $res['comment_clean'] = htmlspecialchars($res['commento'], ENT_QUOTES, 'UTF-8'); //commento is an attribute of my table, where I save the comment
            echo json_encode($res);
        } 
    }
Titania
  • 63
  • 9
  • 2
    Just a note: while you may have (attempted to) protect it from script injection, you've failed to adequately protect it from SQL injection. – ADyson Aug 26 '20 at 09:41
  • "when I insert this tag in a comment in my website"...which bit is the code which does that? As far as we can see here we've only got the saving code, not the displaying code. There could be a problem in either place, potentially. Have you looked in your database, to see whether the data saved into the table is HTML-encoded or not? That would be a good start in narrowing down the root cause of the issue. – ADyson Aug 26 '20 at 09:42
  • To prevent from SQL injection the teacher told us to use the quote function, which I have used – Titania Aug 26 '20 at 09:43
  • "the teacher told us to use the quote function"...well then your teacher is about 15 years out of date. You need to use prepared statements and parameters, that's the only way to protect against SQL injection fully. Which database library are you using? PDO maybe, or mysqli? You can easily find instructions for using parameterised queries with either of those online. – ADyson Aug 26 '20 at 09:43
  • 1
    Well, your teacher is wrong, and you should let him/her know. – Jeto Aug 26 '20 at 09:44
  • Yes, but this is our first website and the course wasn't all about this, so he doesn't pretend very much about security – Titania Aug 26 '20 at 09:45
  • https://stackoverflow.com/questions/22066243/php-does-pdo-quote-safe-from-sql-injection – Kevin Aug 26 '20 at 09:45
  • By the way, in my db the function htmlspecialchar works, only in the code it doesn't work – Titania Aug 26 '20 at 09:46
  • 1
    Anyway regarding the script injection...I can see in your "success" function you're doing `newComment = ""+ comm.val()`, and then putting that into your page. So in this code you're just taking the raw comment value from the input directly. You're not using the encoded version which was saved into the database. – ADyson Aug 26 '20 at 09:46
  • P.S. "he doesn't pretend very much about security"...maybe not, but he shouldn't be teaching you bad habits either. If he simply ignored the topic that's one thing, but actually you mentioned that he specifically told you to do something which is incorrect - that's far worse IMO, because people will assume that's the correct thing to do in future. I agree with Jeto, you should call the teacher out on this, before they pass on incorrect information to any more students. – ADyson Aug 26 '20 at 09:48
  • How can I write this in js: "You're not using the encoded version which was saved into the database"? – Titania Aug 26 '20 at 09:52
  • 1
    It is sadly very common for educators and education establishments to be extremely out of date on IT topics, because IT topics update and renew every 1-2 years whereas other education topics (Woodwork / Sciences / History) do not change much for decades or even centuries, the tutors are not used to having to actually explore *and re-explore* **best practise**. So that you as the student are left in a tough situation; either do what your tutor expects, *or* do Best Practise. It's not a great choice and leads to many IT companies taking experience over grades every time. Good luck – Martin Aug 26 '20 at 09:54
  • 2
    Applying `htmlspecialchars` to the data that gets inserted into the database, is fundamentally wrong to begin with. This should happen when the data is read back from the database, the moment it gets _put into_ the HTML context. But you have not even shown us _that_ part yet, currently the only thing you are giving back from the server to the client is the user name from the session, all other values are still client-side ones to begin with. – CBroe Aug 26 '20 at 09:57
  • You are right, I add the other part of my php code – Titania Aug 26 '20 at 09:57
  • @Titania you have two options for that scenario: 1) try to html-encode the data using Javascript, or 2) return the encoded data from PHP and use that instead. If you want option 1, then a quick google search reveals things like this: https://css-tricks.com/snippets/javascript/htmlentities-for-javascript/ - here's a programmer's tip: as a beginner you're never the first person to have any specific requirements. So in the 21st century a search engine can answer your question very frequently, if you search for simple phrases. – ADyson Aug 26 '20 at 09:58
  • @CBroe that's absolutely correct, but take note also of the code in the "success" function where the server-side code/data is being bypassed entirely and the data injected directly into the page without any santisation at all. But yes there's also likely to be at least one other place where the saved data is also added to the page, so that would need attention too. – ADyson Aug 26 '20 at 10:00

1 Answers1

4

I will list here a few useful answers for both the question you've asked as well as the issues that are obvious from your code:

1)

How to correctly set up your PDO connection -- A good guide to settings to use.

How to use PDO prepared statements - Prepared statements are fundamentally more secure than the previous method of inserting data. You are using PDO but you're not Preparing your statement, so you're almost there....

2)

How to prevent XSS with PHP

NOTE: You should use htmlspecialchars when outputting data, NOT when inputting data into the database.

3)

If you want to allow any HTML in your source input then you should use HTML Purifier on your form data, once its sent back to your server and prior to inserting into the database.

4)

There is a lot of MySQL Best Practise to find and use, and I can't find a nice single post to explain it all, but the one I really need to outline to you is

You should never use the root login to access/save data via the website. ALWAYS use another custom user which has only the limited permissions required.

(If someone has a link to a URL reference for 4 that would be great!)

Good Luck


Yes, but in php or jquery? I understood that this was a php function, so I was thinking about the first part of my php code.. if you could also write the line I have to insert it would be very useful

Your process should be roughly:

Saving Data

  • HTML: Collect data via HTML <form> (etc.), data is sent to the server with a form submit.
  • PHP Optional processing of the data as you require (checking validity, etc.)
  • PHP Use HTMLPurifier or similar, as required to safely remove unwanted HTML.
  • PHP This collected data should be put into a PDO Prepared Statement
  • SQL The data is saved into the database.

Outputting Data

  • PHP Build SQL request for which data you want, typically with an id reference call.
  • PHP Load the data intended to be returned to the Client (the user's browser)
  • PHP Run htmlspecialchars on this data to disable HTML elements
  • HTML data is displayed to the client browser.

I understood the process and I've also read that thread, but I don't know how to write it in my code

Here is an example from your code:

It is also worth noting you should always manually reference all the column names and NOT SELECT * FROM on your SQL query.

    $rows = $db->prepare("SELECT id, name, htmlcode FROM reviews WHERE titolo=:title ORDER BY ora ASC");
    $rows->execute(['title' => $ti]); //SAFE SQL query.
    if((int)$rows->rowCount() === 0){ 
        echo 0;
    } else{
        $res = $rows->fetchALL(PDO::FETCH_ASSOC);
        // Res will be a multilevel array so you need to apply changes to each subarray key. 
        foreach($res as &$resRow){
        //disable HTML. For example only this is saved into a new array value.
            $resRow['commento'] = htmlspecialchars($resRow['commento'], ENT_QUOTES, 'UTF-8'); 
        }
        echo json_encode($res);
    } 

You should also get used to using absolute comparison operators === rather than vague comparison operators == .

Martin
  • 19,815
  • 6
  • 53
  • 104
  • Sorry, but I'm new to these languages and so I have some difficluties, but so I should use htmlspecialchars in the first part of my php code (when I show all my comments)? Can you help me on this point please? – Titania Aug 26 '20 at 10:12
  • @Titania, you should use `htmlspecialchars` when you display/output the HTML data you have in the database. NOT when you save it *to* the database. – Martin Aug 26 '20 at 10:13
  • Yes, but in php or jquery? I understood that this was a php function, so I was thinking about the first part of my php code.. if you could also write the line I have to insert it would be very useful – Titania Aug 26 '20 at 10:16
  • @Titania I have updated my answer, does that give you a better overview of the process? – Martin Aug 26 '20 at 10:23
  • @Titania I highly recommend [reading this thread](https://stackoverflow.com/questions/1996122/how-to-prevent-xss-with-html-php) – Martin Aug 26 '20 at 10:24
  • I understood the process and I've also read that thread, but I don't know how to write it in my code – Titania Aug 26 '20 at 10:28
  • @Titania I have further updated my answer. – Martin Aug 26 '20 at 10:39
  • Thanks a lot, but so I need to use a new attribute on my table? – Titania Aug 26 '20 at 10:51
  • @Titania, no, I do not know all your column names so I used example names only, `id,name, htmlcode` is example names only. you need to apply these to your own code and adjust accordingly. – Martin Aug 26 '20 at 10:55
  • I've tried with your solution (see my edit, later I'm going to follow your suggestions about SQL query), but nothing changes. Am I doing it wrong? – Titania Aug 26 '20 at 12:19
  • @Titania what change did you expect? If you read the `echo json_encode($res);` it will look exactly the same on the output but you will need to `view source` on your web browser to see the character changes (`>` becomes `>` , etc.) – Martin Aug 26 '20 at 13:10
  • @Titania I missed that `fetchAll` will be multidimensional, I have updated my answer code, please review. – Martin Aug 26 '20 at 13:15
  • I've tried with the new code, but nothing changes... – Titania Aug 26 '20 at 13:32
  • @Titania and are you checking the output sorce code, of the `echo` statement? – Martin Aug 26 '20 at 13:35
  • What changes are you expecting? If there is no HTML symbols in the original text `commento` then nothing will change . – Martin Aug 26 '20 at 13:36
  • I expect not to see the alert window when I insert as a comment this tag: `` – Titania Aug 26 '20 at 13:38
  • But I still see the alert window – Titania Aug 26 '20 at 13:38
  • You are echoing ALL values from the `JSON_encode` ; so the original (`$res[0]['commento']` is still there and so will still be perceived as code. Try setting the `comment_clean` array key to the same as `commento` – Martin Aug 26 '20 at 13:54
  • `$d[0] = ""; $d[1] = htmlspecialchars($d[0], ENT_QUOTES, 'UTF-8'); print json_encode($d);` this will show you the diference – Martin Aug 26 '20 at 13:56
  • Ok, I understood what you are saying, but it would be great if you wrote the line for my code; I tried but I've surely made some mistakes – Titania Aug 26 '20 at 14:05
  • @Titania see my edit to my answer – Martin Aug 26 '20 at 14:06
  • now the browser shows me this error: Uncaught SyntaxError: JSON.parse: unexpected character at line 1 column 1 of the JSON data – Titania Aug 26 '20 at 14:09
  • try turning off double encoding on your `htmlspecialchars`, also [read here](https://stackoverflow.com/a/42263361/3536236) – Martin Aug 26 '20 at 14:15
  • Could you please update your code again with the solution to this problem? – Titania Aug 26 '20 at 14:31