Key tampering: what are protective measures if any?

[bartlesby]

I'd like to preface by saying this isn't something that has come up for me personally or likely even many other people. Communities can handle bad actors in house. I'm asking more out of curiosity than urgent need. I apologize if it's already been addressed.

I recently made a plugin to add likes to conversations, which utilizes the Message key, and after getting the core work done, it occurred to me just how easy it is for a user to force whatever data into the key that they want. This isn't a huge problem with keys set on users because the information stored there is only pertinent to them but when it comes to keys used to share information across users, it becomes a potential trainwreck.

Now, when I wrote that plugin, I made a serious goof. I caught it before putting it into action but I was embarrassed that I made it at all. The values I was putting into and taking out from the key were going to be displayed-- as html. I'm sure some of you know where that's going.

For everybody else, here's a very abridged example of what I did wrong. First, the code to set the key:

var addToKey = "user_name_here";
pb.plugin.key(key).set({ object_id: 42, value: addToKey });

Then the code to display it for all users.

var keyString = pb.plugin.key(key).get(42); // returns addToKey
var content = "<a href='doesnt_matter'>" + keyString + "</a>";
$(targetElement).html(content);
So what's wrong there? The user can change addToKey to whatever they like using the browser console. If instead of "user_name_here", they decide to use "<script>console.log('gotcha!');</script>", suddenly every user is executing potentially malicious script.

That's my dumb mistake and I just stripped the tags using regex before putting anything from the key into the DOM.

var keyString = pb.plugin.key(key).get(42); // returns addToKey
keyString = keyString.replace(/(<([^>]+)>)/ig,"");
var content = "<a href='doesnt_matter'>" + keyString + "</a>";
$(targetElement).html(content);
I'm sure there are better ways to go about it but I found that the most convenient. Anyway, after worrying about the malicious I started worrying about the annoying. Perhaps somebody filling up the entire key, putting in false information, messing with parsing, etc. That gets to the crux of my question, of course:

How do you secure your keys and how secure can they be? It seems to me you can't prevent somebody from putting in whatever information they like. Can it be worked around and what are your limitations?

[Peter]

I'm happy this topic has come up.  I've been doing some testing of plugins over the last few days to see what the most common XSS vulnerability was, and it was pretty much always the same apart from one or two that had attribute vulnerabilities.

So XSS is not a small subject, so I will try and keep it light so it's more readable to the general members.

So let's get straight to it.   You can't do anything to prevent the client from modifying your key.  Simple as that.  

Someone with a little knowledge of the ProBoards API can alter the data to whatever they want to a degree (based on data structure and how you read it).  Even if the API has some magical way to prevent you from altering the key data, then I could just build up my own request.

The number one rule is to not trust anyone.​​​​  ​Doesn't matter if it's your best friend from down the road, your sister,  or another staff member (though this depends on level of access to the forum)​​, DO NOT TRUST THEM.​  When you learn this rule, then it becomes a lot easier when writing code, because it will be on your mind.  Whenever I write a plugin that potentially allows input from the client (aka member), or I am outputting something from the key, I automatically starting to think about what I could do to exploit it.

TEST TEST TEST,  and TEST some more.​​​  I'm not perfect, I've let the odd one slip by.  Only reason I find them is because I test my plugins.  I test it being a staff member, a regular member, and sometimes even as a guest (depending on key permissions).  I test different types of vectors, from breaking attributes, to encoding issues, to simply HTML in strings.

Setup your key permissions correctly.  If your plugin is a feature that applies to members and higher, then don't set write permissions to "Everyone".  Same for which key type to use.  If it's something that the member will only see (i.e personal notes), then make it a private key, and you know what... it's private to them, so they can do whatever they like to the data.  Key permissions isn't a solution to fixing XSS vulnerabilities though.

Let's move on to some solutions.

If you know the data type in your key, then always check if it's valid.  Obviously this doesn't work for certain types (i.e strings, because it could contain a vector), but for the simple types like booleans and numbers, it's easy.  For example, if you are storing a number in the key that you output to the page, then cast it to a number, that way you don't need to worry about what the value is because you know the type.

$(".some-container").html("Your Number: " + parseInt(key_data, 10));
You might think that if you done checks for all the types, then you wouldn't need to worry anymore.  Wrong.  Let's say your key data is an array.  I can alter the elements of the array.  Same goes for objects, I can alter the value of the properties.  So while data type checking works in some instances, it doesn't with other types, so this where you need to start outputting the content to the page safely.

You are sort of on the right track with your solution, however why reinvent the wheel when there are solutions out there already?

Let me introduce you to two of my friends:

First up we have escape_html.  Very useful as it covers pretty much everything that could cause you a problem.  I highly recommend you use it on all outputs.  It does come with a small issue that probably won't bother you, and that is over encoding.

pb.text.escape_html(content)
Second, to solve the over encoding issue, we can give escape_html a buddy.  So you would decode and then encode to prevent the over encoding problems (think display names).

function unescape_html(str){
    return str
        .replace(/"/g, '"')
        .replace(/'/g, "'")
        .replace(/&lt;/g, '<')
        .replace(/&gt;/g, '>')
        .replace(/&amp;/g, '&');
}
There are quite a few solutions, a personal fav of mine for non attribute content is from here, and this is also a good read on the different solutions and what issues they have:

stackoverflow.com/questions/1219860/html-encoding-lost-when-attribute-read-from-input-field

Each solution has it up and downs, but you can't really go wrong with what is already available to you, and that is the one supplied to you by ProBoards in the API.

Finally let me give you this:

www.owasp.org/index.php/XSS_(Cross_Site_Scripting)_Prevention_Cheat_Sheet

Hope that helps, have fun