2
\$\begingroup\$

My application will accept textarea content that is submitted by a user, and i would like some people to review my code to make sure there is no security vulnerability such as XSS.

My mySQL column that will save this information is a column of type TEXT and is not required and nullable.

When storing the data to database, my script is doing the following:

// to avoid inserting html tags in the database
$input = str_replace(["<", ">"],"", $_POST['userinput'])

// to avoid saving problematic characters such as quotes 
$cleanInput = htmlentities(input , ENT_QUOTES)

// store the content
$addPostStmt = $conn -> prepare("
  INSERT INTO posts(description) VALUES ( ?)
");

$addPostStmt -> bind_param("s", $cleanInput); 
$addPostStmtExecute = $addPostStmt -> execute();

When presenting the data to the user, the script is doing the following:

<?php echo htmlspecialchars(html_entity_decode($post['description']), ENT_SUBSTITUTE); ?>
\$\endgroup\$
  • \$\begingroup\$ What's the logic behind doing htmlspecialchars/ html_entity_decode/ then htmlspecialchars again? \$\endgroup\$ – Your Common Sense Aug 10 at 16:43
  • \$\begingroup\$ @YourCommonSense not sure what you mean by htmlspecialchars again, i am only doing it one, after decoding the html entities. its mostly incase something slips through \$\endgroup\$ – pabloBar Aug 10 at 16:55
  • \$\begingroup\$ htmlspecialchars and htmlentities is virtually the same, so what's the point doing the same job twice? \$\endgroup\$ – Your Common Sense Aug 10 at 17:00
  • \$\begingroup\$ Or to put it the other way, what's the point in doing entity encode and then decode? \$\endgroup\$ – Your Common Sense Aug 10 at 17:02
6
\$\begingroup\$

No, don't do that. You seem to be filtering and escaping values out of paranoia rather than understanding what exactly would lead to a vulnerability. As a result, you are corrupting your data.

A well designed application should use the database to store the value that the user typed into the textarea, not some mangled representation of it. If you mangle the data like that before storing it, then:

  • Certain characters that the user typed get dropped. (What if the user input is x + 3 < 5? The data would no longer make sense after you drop the < character.)
  • Your database is not reliably searchable. (What if the user input is She said "yes!"? Then you would store a value in the database with &quot; in it.)
  • If you arbitrarily apply escaping to string just in case, then you'll have a hard time keeping track of how to unescape it correctly when regurgitating the data. (This often leads to bugs where the user sees garbage like his &amp; hers, or even worse, his &amp;amp; hers.)

What's the right way? Don't mangle the data; just store it faithfully:

// store the content
$addPostStmt = $conn -> prepare("
  INSERT INTO posts(description) VALUES (?)
");

$addPostStmt -> bind_param("s", $_POST['userinput']); 
$addPostStmtExecute = $addPostStmt -> execute();

When outputting the data as HTML, apply HTML escaping:

<th>Description:</th><td><?php echo htmlspecialchars($description); ?></td>
\$\endgroup\$
  • \$\begingroup\$ escaping values out of paranoia, that is correct tbh. As i dont have a alot of experience with php and mysql, i chose to be very careful to what to add to my database. 1. < and > does not get used usually when writing content like articles and description. So i thought it would be safer to remove it. The content wont be related to math but i see your point. But instead of filtering out these character, is ok to filter <script instead in case someday i forgot to use htmlspecialchars \$\endgroup\$ – pabloBar Aug 10 at 21:35
  • \$\begingroup\$ 2. and 3. True and i completely agree, but the reason i chose to do it like that is because i was not really certain if there where any magical/non-visible character that might cause a problem with sql. But as long i am using prepared statments i guess i should be fine. Are there edge case scenarios that i should be aware of, where having quote characters in the database might be dangerous? \$\endgroup\$ – pabloBar Aug 10 at 21:36
  • 2
    \$\begingroup\$ Nothing more to worry about. You're using the database API correctly. Just trust that it does the right thing, and don't apply any extra data-corrupting transformations. As long you call htmlspecialchars() when outputting the string as HTML, that will correctly take care of XSS concerns. \$\endgroup\$ – 200_success Aug 10 at 23:06
  • \$\begingroup\$ just a side question, why did you drop the ENT_SUBSTITUTE flag ? \$\endgroup\$ – pabloBar Aug 11 at 20:30
  • \$\begingroup\$ Because html_entity_decode() should be unnecessary altogether, if you don't apply a superfluous round of encoding with htmlentities(). \$\endgroup\$ – 200_success Aug 12 at 2:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.