Escaping in messages
Mark Fowler
JavaScript or HTML escaping. This means that if someone does
something like this:
ADDMESSAGE demo "); alert("I IZ IN UR BROWSER STEALING UR COOKIES
A major security breach occurs with meteor.js as this is what is sent
to the browser:
<script>p(1,"demo",""); alert("I IZ IN UR BROWSER STEALING UR
COOKIES");</script>
Now, I realise that only privileged applications should be able to
connect to the Controller port and send messages, but you're currently
putting the burden of understanding of what you need to escape on the
application that sends the message, whereas it really should have no
understanding of this stuff and rely on the transport mechanism to
transport the message sensibly.
As such, I've hacked my local copy to do some escaping of stuff for me
in Messages.pm. And I've attached a patch at the end of this message.
Thanks.
Mark.
http://twoshortplanks.com/
Index: Meteor/Message.pm
===================================================================
--- Meteor/Message.pm (revision 24)
+++ Meteor/Message.pm (working copy)
@@ -114,7 +114,7 @@
}
elsif(exists($self->{$1}))
{
- $self->{$1};
+ $self->munge($self->{$1})
}
else
{
@@ -125,5 +125,27 @@
$msg;
}
+sub munge {
+ my $self=shift;
+ my $string=shift;
+
+ # normally we're in a <script>p("foo", "$string");</script> so we
want our
+ # messages not to break the "" that holds our string
+ $string =~ s{\\}{\\\\}g if $::CONF{'MessageEscapeString'};
+ $string =~ s{"}{\\"}g if $::CONF{'MessageEscapeString'};
+
+ # normally we're in a <script>p("foo", "$string");</script> so we
don't
+ # want a </script> in our string to end the script block. We
escape
+ # any occurance of the word "script" because browers are weird and
let
+ # you do all kinds of odd things with spaces around the tag name.
It's
+ # just safer this way
+ $string =~ s{(scr)(ipt)}{$1"+"$2}gi if
$::CONF{'MessageEscapeEndScript'};
+
+ # as we're running under HTML we want our '&' to be properly
escaped
+ $string =~ s{&}{&}g if
$::CONF{'MessageEscapeAmpersand'};
+
+ $string;
+}
+
1;
############################################################################EOF
\ No newline at end of file
Index: Meteor/Config.pm
===================================================================
--- Meteor/Config.pm (revision 24)
+++ Meteor/Config.pm (working copy)
@@ -84,6 +84,15 @@
'Message template, ~text~, ~id~, ~channel~ and ~timestamp~ will be
replaced by the appropriate values',
MessageTemplate =>
'<script>p(~id~,"~channel~","~text~");</script>\r\n',
+q{Should we escape '"' and '\\' when we replace values in the message
template?},
+ MessageEscapeString => 1,
+
+q{Should we escape '&' with '&' when we replace values in the
message template?},
+ MessageEscapeAmpersand => 1,
+
+q{Should we escape 'script' into 'scr"+"ipt' when we replace values
in the message template to avoid a rouge </script>?},
+ MessageEscapeEndScript => 1,
+
'Interval at which PingMessage is sent to all persistent and
identified subscriber connections (ie those including
id=someuniqueidentifier in their request, and not specifying
persist=0). Must be at least 3 if set higher than zero. Set to zero to
disable.',
PingInterval => 5,
andrew...@gmail.com
the PHP magic_quotes_gpc behaviour, in that it is designed to stop
people being stupid, but in doing so allows them to be stupid. In
this case it would allow controllers to submit messages without
worrying about escaping them - but I think it's the controller's job
to escape messages appopriately for the particular format of data that
it's dealing with.
After all, Meteor is only a streaming framework. Admittedly it will
only work with the clientside JS library if it is configured to send
messages with the JS wrapper, but equally it could be used to stream
XML, JSON, some custom text-based format... it obviously depends on
what kind of client you're connecting to it.
The idea is that the controllers are priviledged apps, so if there is
a requirement for non-priviledged users to be able to arbitrarily
submit messages, the escaping should be done by the priviledged
controller that injects the message, not by Meteor itself.
In practice I've typically URLencoded the messages prior to sending
them to meteor, and then decoded them with decodeURIComponent on the
JS end.
Quite willing to be persuaded on this, but that's my view as it
stands.
Andrew
On Jun 24, 10:48 am, Mark Fowler <2shortpla...@gmail.com> wrote:
> Right now, messages are sent raw by the server with no respect for
> JavaScript or HTML escaping. This means that if someone does
> something like this:
>
> ADDMESSAGE demo "); alert("I IZ IN UR BROWSER STEALING UR COOKIES
>
> A major security breach occurs with meteor.js as this is what is sent
> to the browser:
>
> <script>p(1,"demo",""); alert("I IZ IN UR BROWSER STEALING UR
> COOKIES");</script>
>
> Now, I realise that only privileged applications should be able to
> connect to the Controller port and send messages, but you're currently
> putting the burden of understanding of what you need to escape on the
> application that sends the message, whereas it really should have no
> understanding of this stuff and rely on the transport mechanism to
> transport the message sensibly.
>
> As such, I've hacked my local copy to do some escaping of stuff for me
> in Messages.pm. And I've attached a patch at the end of this message.
>
> Thanks.
>
> Mark.http://twoshortplanks.com/