A potential XSS vulnerability in Stripes?

[Alan Burlison]

I have some code like this:

ctx.getMessages().add(new SimpleMessage(
   "Record ''{0}'' deleted", recordName));

If recordName contains HTML characters they are output unescaped by the
<stripes:messages> tag.  This contrasts with field validation errors,
which are correctly escaped.  Shouldn't all the error handling and
messaging stuff work the same way, and escape HTML characters?

--
Alan Burlison
--

-------------------------------------------------------------------------
Sponsored by: SourceForge.net Community Choice Awards: VOTE NOW!
Studies have shown that voting for your favorite open source project,
along with a healthy diet, reduces your potential for chronic lameness
and boredom. Vote Now at http://www.sourceforge.net/community/cca08
_______________________________________________
Stripes-users mailing list
Stripes-users@...
https://lists.sourceforge.net/lists/listinfo/stripes-users

[Alan Burlison]

Should I log this as a bug?

> I have some code like this:
>
> ctx.getMessages().add(new SimpleMessage(
>    "Record ''{0}'' deleted", recordName));
>
> If recordName contains HTML characters they are output unescaped by the
> <stripes:messages> tag.  This contrasts with field validation errors,
> which are correctly escaped.  Shouldn't all the error handling and
> messaging stuff work the same way, and escape HTML characters?

--
Alan Burlison
--

-------------------------------------------------------------------------
Sponsored by: SourceForge.net Community Choice Awards: VOTE NOW!
Studies have shown that voting for your favorite open source project,
along with a healthy diet, reduces your potential for chronic lameness
and boredom. Vote Now at http://www.sourceforge.net/community/cca08
_______________________________________________
Stripes-users mailing list
Stripes-users@...
https://lists.sourceforge.net/lists/listinfo/stripes-users

[Tim Fennell-3]

Hey Alan,

I'm not sure this is a bug.  I'm not saying it isn't, I'm just not  
sure it is either.  You are supplying recordName to the message  
right?  If we escaped all the parameters to a message then you would  
never be able to supply an HTML character in a message.  If you  
wanted to write (god knows why, but whatever):
        new SimpleMessage("Error: {0}", "You sir are a <bold>moron</bold>")
you wouldn't be able to.

The only things that the validation errors encode by default (I  
believe) are the user input which isn't supplied by the progammer,  
but directly by the user.

We could, to make life easier, provide an optional method or  
something to switch on encoding, but I don't think we'd want to do it  
all the time.  Thoughts?

-t

On Jul 3, 2008, at 8:43 AM, Alan Burlison wrote:

> Should I log this as a bug?
>
>> I have some code like this:
>>
>> ctx.getMessages().add(new SimpleMessage(
>>    "Record ''{0}'' deleted", recordName));
>>
>> If recordName contains HTML characters they are output unescaped  
>> by the
>> <stripes:messages> tag.  This contrasts with field validation errors,
>> which are correctly escaped.  Shouldn't all the error handling and
>> messaging stuff work the same way, and escape HTML characters?
>
> --
> Alan Burlison
> --
>
> ----------------------------------------------------------------------
> ---
> Sponsored by: SourceForge.net Community Choice Awards: VOTE NOW!
> Studies have shown that voting for your favorite open source project,
> along with a healthy diet, reduces your potential for chronic lameness
> and boredom. Vote Now at http://www.sourceforge.net/community/cca08
> _______________________________________________
> Stripes-users mailing list
> Stripes-users@...
> https://lists.sourceforge.net/lists/listinfo/stripes-users

-------------------------------------------------------------------------
Sponsored by: SourceForge.net Community Choice Awards: VOTE NOW!
Studies have shown that voting for your favorite open source project,
along with a healthy diet, reduces your potential for chronic lameness
and boredom. Vote Now at http://www.sourceforge.net/community/cca08
_______________________________________________
Stripes-users mailing list
Stripes-users@...
https://lists.sourceforge.net/lists/listinfo/stripes-users

[Freddy D.]

The way I understood it is that although allowing markup in a
message is often not a good idea, it shouldn't be impossible
either, e.g. if you're running a blog that allows commenters
to use markup.

I don't know if adding a method to switch on encoding is
necessary, when you can simply do:

ctx.getMessages().add(new SimpleMessage(
    "Record ''{0}'' deleted", HtmlUtil.encode(recordName)));

What do you think?

Cheers,
Freddy

-
- Hey Alan,
-
- I'm not sure this is a bug.  I'm not saying it isn't, I'm just not  
- sure it is either.  You are supplying recordName to the message  
- right?  If we escaped all the parameters to a message then you would  
- never be able to supply an HTML character in a message.  If you  
- wanted to write (god knows why, but whatever):
- new SimpleMessage("Error: {0}", "You sir are a <bold-moron</bold-")
- you wouldn't be able to.
-
- The only things that the validation errors encode by default (I  
- believe) are the user input which isn't supplied by the progammer,  
- but directly by the user.
-
- We could, to make life easier, provide an optional method or  
- something to switch on encoding, but I don't think we'd want to do it  
- all the time.  Thoughts?
-
- -t
-
- On Jul 3, 2008, at 8:43 AM, Alan Burlison wrote:
-
- - Should I log this as a bug?
- -
- -- I have some code like this:
- --
- -- ctx.getMessages().add(new SimpleMessage(
- --    "Record ''{0}'' deleted", recordName));
- --
- -- If recordName contains HTML characters they are output unescaped  
- -- by the
- -- <stripes:messages- tag.  This contrasts with field validation errors,
- -- which are correctly escaped.  Shouldn't all the error handling and
- -- messaging stuff work the same way, and escape HTML characters?
- -
- - --
- - Alan Burlison
- - --
- -
- - ----------------------------------------------------------------------
- - ---
- - Sponsored by: SourceForge.net Community Choice Awards: VOTE NOW!
- - Studies have shown that voting for your favorite open source project,
- - along with a healthy diet, reduces your potential for chronic lameness
- - and boredom. Vote Now at http://www.sourceforge.net/community/cca08
- - _______________________________________________
- - Stripes-users mailing list
- - Stripes-users@...
- - https://lists.sourceforge.net/lists/listinfo/stripes-users
-
- -------------------------------------------------------------------------
- Sponsored by: SourceForge.net Community Choice Awards: VOTE NOW!
- Studies have shown that voting for your favorite open source project,
- along with a healthy diet, reduces your potential for chronic lameness
- and boredom. Vote Now at http://www.sourceforge.net/community/cca08
-





-------------------------------------------------------------------------
Sponsored by: SourceForge.net Community Choice Awards: VOTE NOW!
Studies have shown that voting for your favorite open source project,
along with a healthy diet, reduces your potential for chronic lameness
and boredom. Vote Now at http://www.sourceforge.net/community/cca08
_______________________________________________
Stripes-users mailing list
Stripes-users@...
https://lists.sourceforge.net/lists/listinfo/stripes-users

[David G Friedman]

Isn't XSS usually an attack for information which changes often (mutable
data, blog comments, etc.) and which should NOT normally include your
site's system messages?  So, if your message files get modified to
include XSS code then you either have an employee you should FIRE or a
HACKER which is a bigger problem in and of itself.

As you can see from my above point of view, I happen to view this as a
non-issue as well as a useful feature as it is implemented now.

Regards,
David G. Friedman

>>> I have some code like this:
>>>
>>> ctx.getMessages().add(new SimpleMessage(
>>>    "Record ''{0}'' deleted", recordName));
>>>
>>> If recordName contains HTML characters they are output unescaped  
>>> by the
>>> <stripes:messages> tag.  This contrasts with field validation errors,
>>> which are correctly escaped.  Shouldn't all the error handling and
>>> messaging stuff work the same way, and escape HTML characters?
>>>      
>> --
>> Alan Burlison
>>    

-------------------------------------------------------------------------
Sponsored by: SourceForge.net Community Choice Awards: VOTE NOW!
Studies have shown that voting for your favorite open source project,
along with a healthy diet, reduces your potential for chronic lameness
and boredom. Vote Now at http://www.sourceforge.net/community/cca08
_______________________________________________
Stripes-users mailing list
Stripes-users@...
https://lists.sourceforge.net/lists/listinfo/stripes-users

[Alan Burlison]

Tim Fennell wrote:

> I'm not sure this is a bug.  I'm not saying it isn't, I'm just not  
> sure it is either.  You are supplying recordName to the message  
> right?  If we escaped all the parameters to a message then you would  
> never be able to supply an HTML character in a message.  If you  
> wanted to write (god knows why, but whatever):
> new SimpleMessage("Error: {0}", "You sir are a <bold>moron</bold>")
> you wouldn't be able to.

Yeah, I agree, I could think of good reasons why you wouldn't want to do
it all the time - which is why I asked rather than just logging a bug ;-)

> The only things that the validation errors encode by default (I  
> believe) are the user input which isn't supplied by the progammer,  
> but directly by the user.

That makes perfect sense.

> We could, to make life easier, provide an optional method or  
> something to switch on encoding, but I don't think we'd want to do it  
> all the time.  Thoughts?

Or just provide an easy way of doing the encoding, so that you can wrap
just the appropriate parameters when you create the message.

The main place this comes up is in custom validation code, where you are
doing stuff 'by hand' as it were.

--
Alan Burlison
--

-------------------------------------------------------------------------
Sponsored by: SourceForge.net Community Choice Awards: VOTE NOW!
Studies have shown that voting for your favorite open source project,
along with a healthy diet, reduces your potential for chronic lameness
and boredom. Vote Now at http://www.sourceforge.net/community/cca08
_______________________________________________
Stripes-users mailing list
Stripes-users@...
https://lists.sourceforge.net/lists/listinfo/stripes-users

[Alan Burlison]

Freddy D. wrote:

> The way I understood it is that although allowing markup in a
> message is often not a good idea, it shouldn't be impossible
> either, e.g. if you're running a blog that allows commenters
> to use markup.
>
> I don't know if adding a method to switch on encoding is
> necessary, when you can simply do:
>
> ctx.getMessages().add(new SimpleMessage(
>     "Record ''{0}'' deleted", HtmlUtil.encode(recordName)));

                                 ^^^^^^^^
                                 I didn't know about that method...

> What do you think?

In light of the above, I think it just calls for a note in the docs:

"If you are passing user-supplied input into the error and message
classes, make sure you encode them with HtmlUtil.encode(), otherwise you
are leaving yourself vulnerable to XSS attacks."

--
Alan Burlison
--

-------------------------------------------------------------------------
Sponsored by: SourceForge.net Community Choice Awards: VOTE NOW!
Studies have shown that voting for your favorite open source project,
along with a healthy diet, reduces your potential for chronic lameness
and boredom. Vote Now at http://www.sourceforge.net/community/cca08
_______________________________________________
Stripes-users mailing list
Stripes-users@...
https://lists.sourceforge.net/lists/listinfo/stripes-users

[Alan Burlison]

David G Friedman wrote:

> Isn't XSS usually an attack for information which changes often (mutable
> data, blog comments, etc.) and which should NOT normally include your
> site's system messages?  So, if your message files get modified to
> include XSS code then you either have an employee you should FIRE or a
> HACKER which is a bigger problem in and of itself.

XSS can use any mechanism which allows user-supplied input to be fed
back to the user's browser.  Parameterised  error messages are one such
mechanism.

--
Alan Burlison
--

-------------------------------------------------------------------------
Sponsored by: SourceForge.net Community Choice Awards: VOTE NOW!
Studies have shown that voting for your favorite open source project,
along with a healthy diet, reduces your potential for chronic lameness
and boredom. Vote Now at http://www.sourceforge.net/community/cca08
_______________________________________________
Stripes-users mailing list
Stripes-users@...
https://lists.sourceforge.net/lists/listinfo/stripes-users