js - jquery : prevent input from code injection

Question

Context

I'm creating a tiny jQuery script : By clicking on a word, user can erase it and rewrite what he wants.

Original world is in a <span>. When clicked, the <span>is replaced by an <input>. no <form>, no submit button, only this <input>.

My problem

When pressing enter key to validate the <input>, the rest of my code works well, except for javascript injection :

<input type="text" value="<script>alert('Evil Script')</script>">

My question

What is the best method to prevent my <input> from executing this evil script when pressing enter key?

Please forgive my ingenuous question, and thank you for your advice.

My wrong code

jQuery(document).ready(function($) {

    initNom();

    function initNom() {
        var nomPerso = $('#nomPerso');
        var cheminNom = localStorage.getItem('userName');

        montrerNom();

        nomPerso.on('click', function() {
            $(this).replaceWith(
                    $('<input id="nomPerso" type="text" value="" placeholder="' + nomPerso.text() + '">')
            );
            $("#nomPerso")
                    .focus()
                    .on('keypress', function (e) {
                        if(e.which === 13) {

                            e.preventDefault();
                            //Disable textbox to prevent multiple submit
                            $(this).attr("disabled", "disabled");

                            sauverNom();
                            montrerNom();

                            $(this).replaceWith($('<span id="nomPerso">' + localStorage.getItem('userName') + '</span>'));

                            $(this).removeAttr("disabled");

                            initNom();

                        }
                    });
        });

        function sauverNom() {
            var nom = $('#nomPerso').val();

            if (typeof(Storage) !== "undefined" && nom) {
                localStorage.setItem('userName', nom);
            }
        }

        function montrerNom() {
            if (!cheminNom) {
                nomPerso.text('{Inserer_nom}');
            } else {
                nomPerso.text(cheminNom);
            }
        }
    }
});

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>

<div id="nomPerso"><script>alert('Evil Script')</script></div>

Also on CodePen: https://codepen.io/LaBriqueHurlante/pen/mwxjRL

You need to show your code, nobody can tell you what to change when they can't see what you currently have. Create a minimal reproducible example that reproduces the behavior. — Tomalak
– Tomalak, Commented Jul 2, 2017 at 14:48
Whatever method you choose, I would recommend having another layer of protection on the server side incase some malicious user does in fact manage to insert some script. Do what you can on the client side to improve the UX but if you have security considerations you should also sanitize data on the server. — Lix
– Lix, Commented Jul 2, 2017 at 15:04
@Tomalak : Sorry, i didn't want to overload this post. I edited it, it has a link to codepen. — Lbh
– Lbh, Commented Jul 2, 2017 at 15:16
No worries, posting code to a programming website is not overloading, that's business as usual. I've also included the code here. Having your code only on an external site makes your question depend on the link, and that's not useful. For the future, most JS/HTML scenarios be embedded right in the question here on SO, all the tools are there. — Tomalak
– Tomalak, Commented Jul 2, 2017 at 15:20
That being said, look into contentediable: It's closer to what you might want. — Tomalak
– Tomalak, Commented Jul 2, 2017 at 15:21

Alon Eitan · Accepted Answer · 2017-07-02 16:04:32Z

2

Just replace the content with .text() instead of .html() so it will escape all the html tags and display them as HTML entities.

The first example will convert the script tags to html entities, so it will be added to the DOM as

&lt;script&gt;alert('Evil Script')&lt;/script&gt;

GOOD / SAFE Example:

$(function() {
  $('span').on('click', function() {
    $(this).text( $('#replace').val() ); // GOOD
    
    //$(this).html( $('#replace').val() ); // BAD
  });
});

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<h3>Click on a word:</h3>
<hr>
<p>
  <span>Hello</span> <span>World</span>
</p>

<input type="text" id="replace" value="<script>alert('Evil Script')</script>" />

BAD / DANGEROUS example

$(function() {
  $('span').on('click', function() {
    // $(this).text( $('#replace').val() ); // GOOD
    
    $(this).html( $('#replace').val() ); // BAD
  });
});

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<h3>Click on a word:</h3>
<hr>
<p>
  <span>Hello</span> <span>World</span>
</p>

<input type="text" id="replace" value="<script>alert('Evil Script')</script>" />

Now, with your code that you added on your last edit, change:

$(this).replaceWith(
    $('<input id="nomPerso" type="text" value="" placeholder="' + nomPerso.text() + '">')
);

With:

$(this).replaceWith(
    $('<input id="nomPerso" type="text" value="" placeholder="">').attr('placeholder', nomPerso.text())
);

And also change:

$(this).replaceWith($('<span id="nomPerso">' + localStorage.getItem('userName') + '</span>'));

With:

$(this).replaceWith(
    $('<span id="nomPerso"></span>').text( localStorage.getItem('userName') )
);

Those changes will ensure that you're adding elements, and setting their properties with escaped html entities, instead of dumping them unsafely to the DOM: https://codepen.io/anon/pen/JJLBLy

Please note that the first time you run the fiddle it will display the alert because it is embedded in the HTML code itself, but when you change the text to <script>alert('Evil Script')</script> it will be displayed as actual text.

edited Jul 2, 2017 at 16:04

answered Jul 2, 2017 at 14:58

Alon Eitan

12k8 gold badges51 silver badges61 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Lbh Over a year ago

Without your help, I think I would never have found the solution. It works perfectly, you have my eternal gratitude! Thanks you again

Alon Eitan Over a year ago

Thank you, glad to help @Hurlemort :)

Collectives™ on Stack Overflow

js - jquery : prevent input from code injection

Context

My problem

My question

My wrong code

1 Answer 1

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

Context

My problem

My question

My wrong code

1 Answer 1

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related