Sanitize strings to avoid special characters break javascript generated by php

Question

I have a php 'search' script that looks for the requested data in a MySQL database and prints a table. Each row of this table can be modified or deleted by clicking on an icon. When you click on one of these icons a javascript function that shows a display is called.

This is the piece of code:

      while ($row = mysqli_fetch_row($result)) {
            // Define $id
            $id = $row[7];

            // Sanitize output
            $user = htmlentities($row[0]);
            $name = htmlentities($row[1]);
            $surnames = htmlentities($row[2]);
            $email = htmlentities($row[3]);
            $role = htmlentities($row[4]);
            $access = htmlentities($row[5]);
            $center = htmlentities($row[6]);

            $message .= "<tr>
                    <td>" . $user . "</td>" .
                    "<td>" . $name . "</td>" .
                    "<td>" . $surnames . "</td>" .
                    "<td>" . $email . "</td>" .
                    "<td>" . $role . "</td>" .
                    "<td>" . $access . "</td>" .
                    "<td>" . $center . "</td>" .
                    "<td>" .
                        "<input type='image' src='../resources/edit.png' id='edit_" . $user . "' class='edit' onclick=edit_user(\"$user\",\"$name\",\"$surnames\",'$email','$role','$access',\"$center\",'$id') title='Editar'></button>" . 
                    "</td>" .
                    "<td>" .
                        "<input type='image' src='../resources/delete.png' id='delete_" . $user . "' class='delete' onclick=delete_user(\"$user\",'$role') title='Eliminar'></button>" . 
                    "</td>
                </tr>";
        }

This is just part of the table I generate. After all this, I encode the table with json_encode and echo it. The echo is captured by an ajax function that decodes it (JSON.parse) and puts it into a div.

The table is correctly rendered and everything works fine with normal characters, but I have detected I can have some problems if I have quotes, slashes and another meaningfull characters. The strings are showed correctly in the table, so there is no problem with php, but the generated javascript doesn't work with some strings.

For example, if I introduce:

</b>5'"

or:

<b>5'6"</b><br><div

as users, when I click on edit or delete icon I get some errors in javascript console:

Uncaught SyntaxError: Invalid regular expression: missing / home.php:1 Uncaught SyntaxError: missing ) after argument list

Uncaught SyntaxError: missing ) after argument list

Uncaught SyntaxError: Unexpected token ILLEGAL

I have tried with several combination of addslash, replace, htmlentites, htmlspecialchars... but I can't get the right one.

What's the right way to work with this in order to avoid any problem?

Thank you.

EDIT:

I have probed this and it seems to work:

In php I use this function:

function javascript_escape($str) {
$new_str = '';

$str_len = strlen($str);
for($i = 0; $i < $str_len; $i++) {
    $new_str .= '\\x' . dechex(ord(substr($str, $i, 1)));
}

return $new_str;
}

and then I use something like

$('<textarea />').html(user).text()

in javascript to decode the string.

Is this safe against XSS attacks?

At the very least you need to have quotes around the Javascript `onclick` code, otherwise any spaces will cause a syntax error, and it's invalid HTML without enclosing quotes. Then, you'll need to `\` escape any quotes. Try `onclick="edit_user('$user', '$name', ....); return false"` and `$user`, `$name`, and the other variables should be run through `addslashes`. — drew010, Apr 20 '16 at 04:34
A better practice is to use a Javascript event handler instead of printing onclick= directly into the element. See [this question and answer](http://stackoverflow.com/questions/6348494/addeventlistener-vs-onclick) — larsAnders, Apr 20 '16 at 04:35
@dres010 Tried, I get Uncaught SyntaxError: Unexpected token ILLEGAL — vjsp90, Apr 20 '16 at 05:04
@IarsAnders I think I could add eventListeners with the id, but I also have the same problems with the id. It's bad generated, double quotes finish the id, althought I use addsplashes. — vjsp90, Apr 20 '16 at 05:10
@Ziggy Crueltyfree Zeitgeister If I only need the id, yes. But I need to use the strings in the function and print them in some inputs. — vjsp90, Apr 20 '16 at 05:14

Jonathan Gray · Accepted Answer · 2016-04-20T22:17:26.880

First, create an HTML-safe JSON string of the array and modify your code to use a data attribute like so:

      while ($row = mysqli_fetch_row($result)) {
            // Define $id
            $id = $row[7];

            // Sanitize output
            $user = htmlentities($row[0]);
            $name = htmlentities($row[1]);
            $surnames = htmlentities($row[2]);
            $email = htmlentities($row[3]);
            $role = htmlentities($row[4]);
            $access = htmlentities($row[5]);
            $center = htmlentities($row[6]);

            $json_str_edit = htmlentities(json_encode(array($row[0], $row[1], $row[2], $row[3], $row[4], $row[5], $row[6], $id)));
            $json_str_delete = htmlentities(json_encode(array($row[0], $row[4])));

            $message .= "<tr>
                    <td>" . $user . "</td>" .
                    "<td>" . $name . "</td>" .
                    "<td>" . $surnames . "</td>" .
                    "<td>" . $email . "</td>" .
                    "<td>" . $role . "</td>" .
                    "<td>" . $access . "</td>" .
                    "<td>" . $center . "</td>" .
                    "<td>" .
                        "<input type=\"image\" src=\"../resources/edit.png\" id=\"edit_$user\" class=\"edit\" data-user=\"$json_str_edit\" title=\"Editar\"></button>" . 
                    "</td>" .
                    "<td>" .
                        "<input type=\"image\" src=\"../resources/delete.png\" id=\"delete_$user\" class=\"delete\" data-user=\"$json_str_delete\" title=\"Eliminar\"></button>" . 
                    "</td>
                </tr>";
        }

Then create an event listener to catch associated click events in JS like this:

function edit_user(user, name, surnames, email, role, access, center, id) {
    // `this` will refer to the html element involved in the event

    }

function delete_user(user, role) {
    // `this` will refer to the html element involved in the event

    }

document.addEventListener('click', function(event) {
    if(event.target.hasAttribute('data-user')) {
        switch(event.target.className) {
            case 'edit':
                edit_user.apply(event.target, JSON.parse(event.target.dataset.user));
                break;
            case 'delete':
                delete_user.apply(event.target, JSON.parse(event.target.dataset.user));
                break;
            }
        }
    }, false);

Alternatively from the addEventListener method, you can simply add this onclick event listener directly to the element like so (I really don't think it matters in this case):

onclick="edit_user.apply(this, JSON.parse(this.dataset.user))"

FYI It's more common practice to use single quotes in scripts to avoid having to escape the double quote characters. Makes things both cleaner and more standardized.

Thanks, but JSON is not enough. I have tried with quotes and they break the functions. — vjsp90, Apr 20 '16 at 18:46
@vjsp90 Have you even tried my solution? It should definitely work. If your quotes break anything that's only because you're not properly escaping them. — Jonathan Gray, Apr 20 '16 at 18:55
Yes, I have tried it. If you put something'", it doesn´t work even if I use addslashes to escape the strings. — vjsp90, Apr 20 '16 at 19:07
@vjsp90 Then narrow down the real issue and resolve it. You should be able to debug code, no? This answer is to your original question and I shouldn't have to waste my time debugging your code for you. This wasn't meant as a copy and paste answer. This was to show you the correct way of doing what you want to do. — Jonathan Gray, Apr 20 '16 at 19:11
@vjsp90 I have modified my code to properly escape the double quotes in order to be able to use double quotes in the actual HTML. That's how it should be done. — Jonathan Gray, Apr 20 '16 at 19:21
Yes, I solved it using ENT_QUOTES. Just to point out. The only 'problem' is that it generates an output that allows code injection. So if you use it inside html, you are not protected against code injection. If it's going to be used directy inside html(), then it's better to use $user (with htmlEntities() applied), instead of $row[0], (without it). — vjsp90, Apr 20 '16 at 20:50
@vjsp90 You don't need ENT_QUOTES if you use double quotes around the actual attribute in the HTML element. HTMLEntities is an entirely HTML-safe encoder. I simply didn't know that it doesn't encode single quotes by default. This is simply because I've never used single quotes to define HTML attributes. It just looks weird to me. I would have obviously caught the error right away had I tried it myself. My edited answer will pass along values in a safe manner, and additionally in their correct form regardless of type (number, string, boolean, array, etc). — Jonathan Gray, Apr 20 '16 at 21:28
Another comment I would like to make is that you can base64 encode the JSON encoded string instead of using `htmlentities`. This would provide the ultimate protection against code injection. It has a slightly extra CPU cost associated with it but it shouldn't be enough to affect any decisions. You would simply have to run the attribute through the `atob` method in JS to decode the string before running it through `JSON.parse`. — Jonathan Gray, Apr 20 '16 at 21:49

Sanitize strings to avoid special characters break javascript generated by php

1 Answers1