Double quotes in messages are causing JS errors
19 views
Skip to first unread message
dionyziz
Mar 8, 2009, 9:21:11 AM3/8/09
to meteorserver
When posting messages that contain double quotes, it results to a
Javascript error. For example, the following publishing message will
cause a Javascript error on the client:
ADDMESSAGE demo "test here"
Furthermore, if the message is user-determined, such as messages in a
chat box -one of the simplest Comet applications-, leaving it
unfiltered can result to Javascript injection, causing XSS security
problems:
ADDMESSAGE demo "); alert( document.cookie ); ( function ( dummy ) {} )
("
Therefore, if you're using the current version of meteor, make sure
you escape your published strings properly. For example, if you're
using PHP for publishing, pass the message through addslashes() for
security.
My recommendation for this is to implement the addslashes() within the
deamon. The publisher should be agnostic of the delivery mechanism of
the message.
I am providing a patch that fixes this problem by incorporating a
security fix into the deamon. I have posted it as an issue on the
Google code project. I'd love to see it implemented in the trunk:
http://code.google.com/p/meteorserver/issues/detail?id=14
Thank you,
Dionysis Zindros,
Kamibu Development Group.
Javascript error. For example, the following publishing message will
cause a Javascript error on the client:
ADDMESSAGE demo "test here"
Furthermore, if the message is user-determined, such as messages in a
chat box -one of the simplest Comet applications-, leaving it
unfiltered can result to Javascript injection, causing XSS security
problems:
ADDMESSAGE demo "); alert( document.cookie ); ( function ( dummy ) {} )
("
Therefore, if you're using the current version of meteor, make sure
you escape your published strings properly. For example, if you're
using PHP for publishing, pass the message through addslashes() for
security.
My recommendation for this is to implement the addslashes() within the
deamon. The publisher should be agnostic of the delivery mechanism of
the message.
I am providing a patch that fixes this problem by incorporating a
security fix into the deamon. I have posted it as an issue on the
Google code project. I'd love to see it implemented in the trunk:
http://code.google.com/p/meteorserver/issues/detail?id=14
Thank you,
Dionysis Zindros,
Kamibu Development Group.
ekste...@gmail.com
Mar 8, 2009, 11:05:58 AM3/8/09
to meteorserver
dionyziz
Mar 10, 2009, 7:18:05 AM3/10/09
to meteorserver
Hi again,
The support provided for refusing that patch seems inadequate to me.
First off, there is no documentation denoting whether the " or \
characters must be escaped and what characters are allowed in general.
If they aren't to be escaped, the patch should be merged into the
trunk (and documentation should be created stating e.g. that new lines
are not allowed within message strings).
If they SHOULD be escaped in the protocol, I strongly believe that the
daemon must store the ACTUAL data to be delivered inside its local
string variables, not the data with extraneous escape characters. This
means essentially parsing the data after the "ADDMESSAGE channelname"
string and un-escaping special characters to generate the actual
message one desires to post, so that the attribute of the attribute
$self->Text for the particular instance of Message contains the text
the developer intended to deliver, not the string that is to be
outputted or the string that was inputted.
This is so that it can be exported in different formats: For example,
a HEREDOC output format would require different escaping from simple
double quotes. Therefore, it should definitely not be dependent on
what the output mechanism is; i.e. the message storing should be
output agnostic. In this case, the patch should still be applied; and
an additional unescaping (strip slashes) mechanism should be
implemented when parsing the input. Furthermore, the documentation
must be expanded to include which characters should be escaped and
which should not.
Sorry for not searching the discussion group prior to creating this
patch. I'd have saved us both some time.
Thanks you again for reading this and considering it :-)
wrote:
> Has come up before:http://groups.google.com/group/meteorserver/browse_thread/thread/d614...
The support provided for refusing that patch seems inadequate to me.
First off, there is no documentation denoting whether the " or \
characters must be escaped and what characters are allowed in general.
If they aren't to be escaped, the patch should be merged into the
trunk (and documentation should be created stating e.g. that new lines
are not allowed within message strings).
If they SHOULD be escaped in the protocol, I strongly believe that the
daemon must store the ACTUAL data to be delivered inside its local
string variables, not the data with extraneous escape characters. This
means essentially parsing the data after the "ADDMESSAGE channelname"
string and un-escaping special characters to generate the actual
message one desires to post, so that the attribute of the attribute
$self->Text for the particular instance of Message contains the text
the developer intended to deliver, not the string that is to be
outputted or the string that was inputted.
This is so that it can be exported in different formats: For example,
a HEREDOC output format would require different escaping from simple
double quotes. Therefore, it should definitely not be dependent on
what the output mechanism is; i.e. the message storing should be
output agnostic. In this case, the patch should still be applied; and
an additional unescaping (strip slashes) mechanism should be
implemented when parsing the input. Furthermore, the documentation
must be expanded to include which characters should be escaped and
which should not.
Sorry for not searching the discussion group prior to creating this
patch. I'd have saved us both some time.
Thanks you again for reading this and considering it :-)
Dionysis Zindros,
Kamibu Development Group.
On Mar 8, 5:05 pm, "eksterg...@gmail.com" <eksterg...@gmail.com>
Kamibu Development Group.
wrote:
> Has come up before:http://groups.google.com/group/meteorserver/browse_thread/thread/d614...
ekste...@gmail.com
Mar 10, 2009, 4:26:39 PM3/10/09
to meteorserver
I agree about expanding the documentation, however I do no believe
meteor should do the escaping.
As Andrew points out in the other discussion, meteor can be used to
send any kind of message to any kind of subscriber.
Escaping " and \ in a message that doesn't require escaping those
characters, but does require escaping other characters is even less
convenient than the current situation.
What you are asking for may solve *your* "problem", but will cause
more problems for other users.
And in the end, the developer using meteor is most likely in control
of both the controller and the choice of client-library and thus in
the perfect position to decide on how to escape and/or encode the
messages.
In short Meteor *is* output agnostic, which is exactly why the
controller should do the escaping.
meteor should do the escaping.
As Andrew points out in the other discussion, meteor can be used to
send any kind of message to any kind of subscriber.
Escaping " and \ in a message that doesn't require escaping those
characters, but does require escaping other characters is even less
convenient than the current situation.
What you are asking for may solve *your* "problem", but will cause
more problems for other users.
And in the end, the developer using meteor is most likely in control
of both the controller and the choice of client-library and thus in
the perfect position to decide on how to escape and/or encode the
messages.
In short Meteor *is* output agnostic, which is exactly why the
controller should do the escaping.
dionyziz
Mar 11, 2009, 7:02:33 PM3/11/09
to meteorserver
Suppose, now, that you would use two different output mechanisms for
subscribers. One, say, through Javascript, and one in a different
output format. Then these should be escaped differently. This escaping
must be done in the library that outputs the data: The string stored
inside the text attribute of the class must contain the literal
message, not the escaped message. Hence, when inputting data as a
controller, you should use the "Meteor Input Format" while when Meteor
is outputting, it should be using the "JS Output Format" or whatever
output format. The point is that, exactly because you want to have
support for different outputs, the message stored in the class
attribute must not be escaped prior to outputting it. Any escaping
done by the controller must be simply so that Meteor is able to
interpret the literal incoming message i.e. \n could mean a literal
new line.
The same way that a PHP programmer should be mysql_escape_real_string
prior to performing a database query, Meteor should call an addslashes
method before outputting data to Javascript. Then again, the
controller must also send the data in a way that Meteor understands
i.e. no new lines within message texts unless they are encoded using
some mechanism (which can be different from what you use in your
output format).
In general, the input and the output format should be disjoint as
there can be different outputting mechanisms. For being able to write
an extensible client, the same input should be able to be used in any
of the output formats to produce the same literal text.
Regards,
wrote:
subscribers. One, say, through Javascript, and one in a different
output format. Then these should be escaped differently. This escaping
must be done in the library that outputs the data: The string stored
inside the text attribute of the class must contain the literal
message, not the escaped message. Hence, when inputting data as a
controller, you should use the "Meteor Input Format" while when Meteor
is outputting, it should be using the "JS Output Format" or whatever
output format. The point is that, exactly because you want to have
support for different outputs, the message stored in the class
attribute must not be escaped prior to outputting it. Any escaping
done by the controller must be simply so that Meteor is able to
interpret the literal incoming message i.e. \n could mean a literal
new line.
The same way that a PHP programmer should be mysql_escape_real_string
prior to performing a database query, Meteor should call an addslashes
method before outputting data to Javascript. Then again, the
controller must also send the data in a way that Meteor understands
i.e. no new lines within message texts unless they are encoded using
some mechanism (which can be different from what you use in your
output format).
In general, the input and the output format should be disjoint as
there can be different outputting mechanisms. For being able to write
an extensible client, the same input should be able to be used in any
of the output formats to produce the same literal text.
Regards,
Dionysis Zindros,
Kamibu Development Group.
On Mar 10, 10:26 pm, "eksterg...@gmail.com" <eksterg...@gmail.com>
Kamibu Development Group.
wrote:
ekste...@gmail.com
Mar 11, 2009, 8:16:11 PM3/11/09
to meteorserver
Your php & mysql example is wrong.
Php talks to mysql through a socket.
A controller talks to a subscriber through meteor.
Continuing your logic if what php does is the right way to do this
then the controller should do the escaping.
Php talks to mysql through a socket.
A controller talks to a subscriber through meteor.
Continuing your logic if what php does is the right way to do this
then the controller should do the escaping.
dionyziz
Mar 14, 2009, 6:36:59 PM3/14/09
to meteorserver
The controller SHOULD do the escaping indeed to adhere to the
publishing/controller/input protocol in order for meteor to receive
the literal string that it must store.
Meteor, then, having received the escaped message through input, needs
to unescape it to store it properly internally (in a class attribute).
Meteor should never store the escaped message internally, only the
literal message.
Finally, when a transport is established using the subscriber/output
protocol, meteor should again perform an escape BASED ON THE OUTPUT
METHOD which can be different in a per-case basis. For example, a
different escape must be used for Javascript double quotes, a
different escape for Javascript single quotes, a different escape for
PHP serialized objects, and so forth, depending on the context (it is
irrelevant that it happens for the context to be just one at the
moment).
Sorry for the late reply. Thanks for reading! And thank you for
discussing this matter extensively, I think it's triggering important
argument points that should be discussed, even if consensus is not
achieved right away.
Dionysis Zindros,
Kamibu Development Group.
On Mar 12, 2:16 am, "eksterg...@gmail.com" <eksterg...@gmail.com>
wrote:
publishing/controller/input protocol in order for meteor to receive
the literal string that it must store.
Meteor, then, having received the escaped message through input, needs
to unescape it to store it properly internally (in a class attribute).
Meteor should never store the escaped message internally, only the
literal message.
Finally, when a transport is established using the subscriber/output
protocol, meteor should again perform an escape BASED ON THE OUTPUT
METHOD which can be different in a per-case basis. For example, a
different escape must be used for Javascript double quotes, a
different escape for Javascript single quotes, a different escape for
PHP serialized objects, and so forth, depending on the context (it is
irrelevant that it happens for the context to be just one at the
moment).
Sorry for the late reply. Thanks for reading! And thank you for
discussing this matter extensively, I think it's triggering important
argument points that should be discussed, even if consensus is not
achieved right away.
Dionysis Zindros,
Kamibu Development Group.
wrote:
ekste...@gmail.com
Mar 18, 2009, 12:05:29 PM3/18/09
to meteorserver
The controller protocol doesn't require any escaping, meteor shouldn't
unescape before storing.
Meteor should not escape based on the output method.
As Andrew wrote in the other thread meteor should not allow people to
be stupid be trying to prevent them from being stupid. And meteor
doesn't know which client library is used in the first place. Can't
escape according to client-library if you don't have the required
information.
*You* on the other hand chose which client libraries are used, *you*
chose which controllers are used, *you* know which (un)escaping should
be used, therefor *you* need to do the escaping.
Meteor doesn't care what it receives, where it came from and where
it's going. If you want something that does all that then meteor is
not for you.
unescape before storing.
Meteor should not escape based on the output method.
As Andrew wrote in the other thread meteor should not allow people to
be stupid be trying to prevent them from being stupid. And meteor
doesn't know which client library is used in the first place. Can't
escape according to client-library if you don't have the required
information.
*You* on the other hand chose which client libraries are used, *you*
chose which controllers are used, *you* know which (un)escaping should
be used, therefor *you* need to do the escaping.
Meteor doesn't care what it receives, where it came from and where
it's going. If you want something that does all that then meteor is
not for you.
Nico
Mar 18, 2009, 2:12:12 PM3/18/09
to meteor...@googlegroups.com
2009/3/18 ekste...@gmail.com <ekste...@gmail.com>
The controller protocol doesn't require any escaping, meteor shouldn't
unescape before storing.
By the way, does the string escaping cost non negligeable cpu time in perl ? (i don't know perl at all)
dionyziz
Mar 19, 2009, 6:16:55 AM3/19/09
to meteorserver
So you're essentially saying that we shouldn't be able to use one
publisher on one channel to publish to two different subscribers using
two different output mechanisms?
D.
On Mar 18, 6:05 pm, "eksterg...@gmail.com" <eksterg...@gmail.com>
publisher on one channel to publish to two different subscribers using
two different output mechanisms?
D.
On Mar 18, 6:05 pm, "eksterg...@gmail.com" <eksterg...@gmail.com>
dionyziz
Mar 19, 2009, 6:19:29 AM3/19/09
to meteorserver
String escaping and unescaping is very fast (O(N)); it takes no longer
(from a complexity point of view) than assigning the string to a
variable or copying it to an object attribute.
The issue about escaping and unescaping really is about the
architecture of the system, not about performance at all.
On Mar 18, 8:12 pm, Nico <dragoon...@gmail.com> wrote:
> 2009/3/18 eksterg...@gmail.com <eksterg...@gmail.com>
(from a complexity point of view) than assigning the string to a
variable or copying it to an object attribute.
The issue about escaping and unescaping really is about the
architecture of the system, not about performance at all.
On Mar 18, 8:12 pm, Nico <dragoon...@gmail.com> wrote:
> 2009/3/18 eksterg...@gmail.com <eksterg...@gmail.com>
Andrew Betts
Mar 26, 2009, 6:13:51 AM3/26/09
to meteorserver
My view on this is that Meteor should not escape any input - though I
can see that it can be argued that because the JavaScript client
requires the output from Meteor to be valid JavaScript code, it makes
sense that Meteor server should ensure that it is. But you can also
argue that the client places lots of other requirements on the way the
server should behanve, none of which are mandatory at the server end
and are simply an issue of configuration. Yes, it ships configured
for the JS client, but it seems silly to make it's most common
configuration into the only possible configuration simply to make it
very slightly easier to queue new events.
Agree that this is not a performance issue at all.
can see that it can be argued that because the JavaScript client
requires the output from Meteor to be valid JavaScript code, it makes
sense that Meteor server should ensure that it is. But you can also
argue that the client places lots of other requirements on the way the
server should behanve, none of which are mandatory at the server end
and are simply an issue of configuration. Yes, it ships configured
for the JS client, but it seems silly to make it's most common
configuration into the only possible configuration simply to make it
very slightly easier to queue new events.
Agree that this is not a performance issue at all.
dionyziz
Mar 26, 2009, 3:48:23 PM3/26/09
to meteorserver
Andrew,
Thinking about the future, I would expect more output methods to
become available on Meteor, other than Javascript. Your current
architecture does not allow communication to different subscribers of
the same channel that would use different output methods (as this
would require different escaping methods for each output method). If
this is something that one would want to make available, they would
have to use a different channel for each output method (for example,
one for Javascript and a different one for another language X).
I understand your point of view. It'd however seem more logical to me
to allow one channel to be pullable through different transports
depending on the client needs by having the controller publish the
message to the channel, being agnostic of the output method.
Thanks for your time and for reading up,
Thinking about the future, I would expect more output methods to
become available on Meteor, other than Javascript. Your current
architecture does not allow communication to different subscribers of
the same channel that would use different output methods (as this
would require different escaping methods for each output method). If
this is something that one would want to make available, they would
have to use a different channel for each output method (for example,
one for Javascript and a different one for another language X).
I understand your point of view. It'd however seem more logical to me
to allow one channel to be pullable through different transports
depending on the client needs by having the controller publish the
message to the channel, being agnostic of the output method.
Thanks for your time and for reading up,
Dionysis Zindros,
Kamibu Development Group.
Kamibu Development Group.
Kevin Peno
Mar 26, 2009, 4:38:00 PM3/26/09
to meteor...@googlegroups.com
Is it a possibility that Meteor could be expanded to allow to include plug-ins for channel output? Thus, you could escape information for JavaScript (via json) or compile to xml for another client (etc) and the input controller can remain agnostic to the output simply post the raw data. Hopefully this would solve the problems discussed here.
channel=2321312&output=json
channel=2321312&output=xml
channel=2321312&output=customnamehere
Kevin Peno
Metalaxe, LLC
425.582.8139
ke...@metalaxe.com
channel=2321312&output=json
channel=2321312&output=xml
channel=2321312&output=customnamehere
Kevin Peno
Metalaxe, LLC
425.582.8139
ke...@metalaxe.com
Reply all
Reply to author
Forward
0 new messages