-2

I have a basic form to submit comments to a database through PHP and display them on the page via AJAX, and everything works OK but the problem is that simple HTML code like <script>alert('Message')</script> or <h1>Hi!</h1> is displayed valid HTML and is executed by the client as such.

After doing research I've found that the PHP htmlspecialchars() function can prevent XSS code from being executed:

<form method="post" action="<?PHP echo htmlspecialchars($_SERVER['PHP_SELF']);?>" onsubmit="return post();">
    <textarea id="comment"></textarea>
    <input type="submit" value="Submit">
</form>

the PHP document that processes the form is this:

<?php

error_reporting(E_ERROR | E_WARNING | E_PARSE | E_NOTICE);

$host="localhost";
$username="user";
$password="password";
$databasename="comments";

$connect=mysql_connect($host,$username,$password);
$db=mysql_select_db($databasename);

if(isset($_POST['user_comm']))
{
  $comment = mysql_real_escape_string($_POST['user_comm']);
  $insert=mysql_query("insert into comments values('','$comment',CURRENT_TIMESTAMP)");

  $id=mysql_insert_id();

  $select=mysql_query("select comment,post_time from comments where comment='$comment' and id='$id'");

  if($row=mysql_fetch_array($select))
  {
      $comment = mysql_real_escape_string($row['comment']);
    $time=$row['post_time'];
  ?>
    <div class="comment_div"> 
      <p class="comment"><?php echo stripslashes($comment);?></p>   
      <p class="time"><?php echo date('l d M Y - H:i a', strtotime($time));?></p>
      </div>
  <?php
  }
exit;
}

?>

But in my case that isn't working and Javascript code is still executed by the client after the message has been sent. How can I display HTML special characters as plain text on the comments to prevent that?

  • 2
    You've only protected `$_SERVER['PHP_SELF']` from XSS here. You need to use `htmlspecialchars` on *any* piece of data that comes from your users, wherever it's shown on the site. Since this is a comments form, you need to use `htmlspecialchars` on the resulting comment when you display it on your website. (Which wouldn't be in this block of code at all.) – ceejayoz Sep 05 '19 at 21:59
  • 1
    `htmlspecialchars($_SERVER['PHP_SELF']);` doesn't really make any sense - that's not user input, you shouldn't need to protect that. – ADyson Sep 05 '19 at 22:00
  • You should be executing htmlspecialchars on the $_POST variables you receive into your script when the form is submitted. If you need help with that, please show that section of your script. – ADyson Sep 05 '19 at 22:01
  • I updated the question and posted all the PHP code. – Andres Guzman Sep 05 '19 at 22:09
  • What if remove all HTML tags? for that use strip_tags – Eugene Sep 06 '19 at 02:58
  • 3
    Possible duplicate of [Why shouldn't I use mysql\_\* functions in PHP?](https://stackoverflow.com/questions/12859942/why-shouldnt-i-use-mysql-functions-in-php) – Dharman Sep 06 '19 at 03:30
  • Why are you using the long-deprecated `mysql_` code library? It was discontinued many years ago and removed entirely in PHP7. No new code should be written using this library. It leaves you vulnerable to SQL injection attacks (due to the lack of parameterised query support) and potentially other unpatched vulnerabilities. Switch to using `mysqli` or `PDO` as soon as possible, and then learn how to write parameterised queries to protect your data from malicious input. See http://bobby-tables.com for a simple explanation of the risks and some sample PHP code to write queries safely. – ADyson Sep 06 '19 at 09:24
  • @ADyson `PHP_SELF` is attackable. It's generated from client-side data. https://www.webadminblog.com/index.php/2010/02/23/a-xss-vulnerability-in-almost-every-php-form-ive-ever-written/ – ceejayoz Sep 06 '19 at 12:20
  • @ceejayoz thanks, good point. I didn't know that. I take back that remark then. Will amend my answer below. – ADyson Sep 06 '19 at 13:26

2 Answers2

0

htmlspecialchars() replaces special characters by their html code, like < by &lt;

Protecting the action of the form has no sense, protect the display of your comment :

<p class="comment"><?php echo htmlspecialchars($comment);?></p>
iguypouf
  • 770
  • 4
  • 15
  • Better to do it when you save the value to the database don't you think? Then you don't waste time doing the same operation again and again every time it gets displayed – ADyson Sep 06 '19 at 06:10
  • Protecting the action **does** make sense in this case. XSS in `PHP_SELF` is very possible. https://www.webadminblog.com/index.php/2010/02/23/a-xss-vulnerability-in-almost-every-php-form-ive-ever-written/ (Ditching `PHP_SELF` entirely is a better approach, though.) – ceejayoz Sep 06 '19 at 12:20
  • @ceejayoz Of course yes, but not to resolve the problem of the OP. – iguypouf Sep 06 '19 at 20:47
  • @ADyson it depends if comment may be an html sequence only protected for display, but needed in another scope, and then has to be stored directly like that – iguypouf Sep 06 '19 at 20:47
  • @iguypouf I'm disagreeing with "protecting the action of the form has no sense". OP is correct to protect it. OP just has a *different* XSS issue to tackle remaining, in the same way. – ceejayoz Sep 06 '19 at 21:54
0

htmlspecialchars() replaces HTML special characters with an encoded version, for example < is replaced by &lt;

As well as protecting your form's "action", you also need to use this to encode any input variables whose data might later be injected into HTML on the screen.

A good time to encode the values would be just before you insert them into the database, so that any time you come to use that data later it can no longer be a danger.

So in your example for you just have the "comment" field, therefore you can encode it like this just before you prepare it for database insertion:

$comment = htmlspecialchars($_POST['user_comm']);
$comment = mysql_real_escape_string($comment);

N.B. Please heed my comment (in the main comments section, above) regarding the use of mysql_query and associated functions. You should not be using these for any new code, and for old code you should be planning to replace it. Switch to a supported library as soon as possible.

ADyson
  • 57,178
  • 14
  • 51
  • 63