Web Sockets with server side logic (2)

This post is more than 2 years old.

A few days ago I posted an update to my websocket chat demo that talked about associating a CFC with the web socket to perform server side operations. While testing the chat, a user (hope he reads this and chimes in to take credit) noted another security issue with the code. I had blogged on this topic already, specifically how my chat handler was escaping HTML but could be bypassed easily enough. The user found another hole though. Let's examine it, and then I'll demonstrate the fix.

When the chat button is pressed, the following code is run:

$("#sendmessagebutton").click(function() { var txt = $.trim($("#newmessage").val()); if(txt == "") return; msg = { type: "chat", username: username, chat: txt }; chatWS.publish("chat",msg); $("#newmessage").val(""); });

I've removed my HTML escaping code since the server handles it. But pay attention to the message payload. It contains a type, a username, and a chat. The username value is set after you sign in. It's a simple global JavaScript variable. It's also trivial to modify. Just create your own structure and pass it to the web socket object:

chatWS.publish("chat", {type:"chat",username:"Bob Newhart", chat:"Howdy"});

The server will gladly accept that and pass it along to others. Not good. Luckily there is a simple enough fix for this. My first change was to remove the username from the packet completely.

$("#sendmessagebutton").click(function() { var txt = $.trim($("#newmessage").val()); if(txt == "") return; msg = { type: "chat", chat: txt }; chatWS.publish("chat",msg); $("#newmessage").val(""); });

If you remember, we had a CFC associated with our web socket that was handling a variety of tasks. One of them supported stripping HTML. Here is the original method:

public any function beforeSendMessage(any message, Struct subscriberInfo) { if(structKeyExists(message, "type") && message.type == "chat") message.chat=rereplace(message.chat, "<.*?>","","all"); return message; }

Notice the second argument we didn't use? This a structure of data associated with the client. We modified this a bit on our initial subscription to include our username. That means we can make use of it again:

message["username"]=arguments.subscriberInfo.userinfo.username;

This will now get returned in our packet. Check it our yourself below. I've included a zip of the code. (And this is my last chat demo. Honest.)

OOPS!

Turns out I have a critical mistake in my fix, but it's one of those awesome screwups that lead to learning. As soon as I posted my demo, a user noted that his chats were being marked as coming from me. I had no idea why. I then modified my CFC to do a bit of logging:

var myfile = fileOpen(expandPath("./log.txt"),"append"); fileWriteLine(myfile,serializejson(message) & "----" & serializejson(subscriberInfo));

I saw this in the log file:

{"chat":"TestAlphaOne","type":"chat"}----{"userinfo":{"username":"Ray"},"connectioninfo":{"connectiontime":"March, 02 2012 09:56:18","clientid":1511146919,"authenticated":false},"channelname":"chat"}

{"chat":"TestAlphaOne","type":"chat"}----{"userinfo":{"username":"chk"},"connectioninfo":{"connectiontime":"March, 02 2012 09:57:49","clientid":542107549,"authenticated":false},"channelname":"chat"}

At the time of this test, there were two users. My one message was sent out 2 times. So this is interesting. To me, I thought beforeSendMessage was called once, but it's actually called N times, one for each listener. That's kind of cool. It means you could - possibly - stop a message from going to one user. Of course, it totally breaks my code.

One possible fix would be to simply see if USERNAME existed in the message packet. But if I did that, an enterprising hacker would simply supply it.

When I figure this out, I'll post again.

SECOND EDIT

Woot! I figured it out. Turns out I should have been using beforePublish. It makes total sense once you remember it exists. It also makes more sense to have my HTML "clean" there too. Things got a bit complex though.

beforePublish is sent a structure too, but it only contains the clientinfo packet. It does not contain the custom information added by the front-end code. I'm thinking this is for security. But, we have the clientid value and we have a server-side function, wsGetSubscribers. If we combine the two, we can create a way to get the proper username:

public any function beforePublish(any message, Struct publisherInfo) {

if(structKeyExists(message, "type") && message.type == "chat") {
	//gets the user list, this is an array of names only
  	var users = getUserList();
  	var myclientid = publisherInfo.connectioninfo.clientid;
	  	  	
  	var me = users[arrayFind(wsGetSubscribers('chat'), function(i) {
  		return (i.clientid == myclientid);
  	})];
  	  	
	message.chat=rereplace(message.chat, "&lt;.*?&gt;","","all");
  	  	
  	message["username"]=me;
}
  	
return message;

}

Does that logic make sense? Basically we are just comparing clientids. I've restored the demo so have at it!

Download attached file.

Raymond Camden's Picture

About Raymond Camden

Raymond is a senior developer evangelist for Adobe. He focuses on document services, JavaScript, and enterprise cat demos. If you like this article, please consider visiting my Amazon Wishlist or donating via PayPal to show your support. You can even buy me a coffee!

Lafayette, LA https://www.raymondcamden.com

Archived Comments

Comment 1 by Salvatore Fusto posted on 3/3/2012 at 2:05 PM

Ray,
very interesting posts: can you add a feature so a user can log out from the chat?
the logout would update the chat users list.
Thanks and regards

Comment 2 by Raymond Camden posted on 3/3/2012 at 5:31 PM

There is support for disconnecting. Also, the server is _supposed_ to notice when you close the connection. That part is broken.

So basically, you can do it manually, and it will also happen automatically in the next update.

Comment 3 by Matt Bram posted on 5/9/2012 at 8:45 AM

Hey Ray,

I'm banging my head against the wall trying to get wsPublish() to work while using coldbox. I was just curious to see if you've experimented with websockets and front end controllers. I noticed that someone posted a bug on the bug report at Adobe Labs, which dealt with Java Concurrency. So, I think that it is either breaking because of the framework interception on the Application.cfc, or it could be because the framework uses multiple threads. Taking a stab at it...
Thanks!
Matt B

Comment 4 by Raymond Camden posted on 5/9/2012 at 3:37 PM

Nope, I've not done any WS stuff with MVC frameworks. If I can do a demo today with it, I will.

Comment 5 by Raymond Camden posted on 5/10/2012 at 6:18 PM

Btw - I'm booked like crazy. If you can send me a ColdBox app that doesn't have any database dependencies and shows you trying to broadcast a WS, I can try running it here on my latest build. Not trying to be lazy, but...

Comment 6 by Matt Bram posted on 5/11/2012 at 6:45 PM

I think that this may have something to do with Apache rewrite rules. I'm exploring now and will post back. Do you know if there is anyway to get an engineering brief from Adobe on the lifecycle of the websocket request? I wouldn't even know who to ask :)

Thanks for your help!
Matt B

Comment 7 by Raymond Camden posted on 5/11/2012 at 6:58 PM

You could try the main forums at Adobe.

Comment 8 by jeff horne posted on 4/3/2013 at 5:28 PM

This demo gets the json parse error I commented about on another blog entry.

SyntaxError: JSON.parse: unexpected character

var data = JSON.parse(message.data); Line 67

Comment 9 by Raymond Camden posted on 4/3/2013 at 5:37 PM

This reflects a change in WebSockets from pre-release to release. You no longer need to JSON parse the result. Simple remove JSON.parse().

Comment 10 by jeff horne posted on 4/4/2013 at 2:19 AM

Fantastic. Thanks Ray.