With my latest commit (rev 742352) I've finished a first pass on a few
things for this. My commit log for that is included below (and has
some important notes).
Now in place are:
1. canonicalization for all incoming parameters
2. encoding of output for all String fields in forms, and for all
String objects in FTL files (use something other than a String object,
like a StringBuffer or something if you don't want encoding)
3. validation of input in the Service Engine to restrict allowed HTML,
with three options for the allow-html attribute including none
(default), safe (antisamy-based), and any (no checking of HTML input)
Which these in place we protect from XSS and related attacks in a few
ways, and most data should be validated in input and encoded on output
so that it is not interpreted by the browser as HTML. This is also a
general good thing as users won't accidently type in characters that
mess up the HTML either.
There is still potential for security holes, especially when the main
(ie best practice recommended) OFBiz tools are not used. Just using
the ControlServlet, the Screen Widget and the Service Engine should
handle most of these issues now (unless someone explicitly disables
these tools).
There are also a bunch of places still where HTML should be allowed,
at least the AntiSamy "safe" HTML, but currently isn't.
I've spent dozens of hours on this stuff now and done a LOT of
testing, but I know there is a lot more testing and refinement to do.
We could use help from anyone listening with the following things:
1. anything that used to work but now doesn't, especially funny text
on screens or complaints about HTML related things
2. security vulnerability tests: now we want to hit the public facing
(ecommerce, cmssite, etc) apps and the back-end apps to check as many
vulnerabilities as we can
3. requests for loosening up in certain places that are now by default
too strict (probably because they haven't been reviewed yet with these
new defaults in place)
Help with these things would be greatly appreciated! Also, thanks for
the help so far from those who have commented and been patient and such.
Going forward, some things I still want to work on are dynamic session
tokens to use in addition to a jsessionid in order to prevent session
hijacking, and also form submission tokens to avoid form hijacking
(just in case XSS slips in, etc) and also to avoid duplicate
submission of forms. As usual when these happen is another question!
Things have been piling up as I pushed on these... :(
-David
==================================
Date: Mon Feb 9 09:34:34 2009
New Revision: 742352
URL: http://svn.apache.org/viewvc?rev=742352&view=rev
Log:
Added new allow-html tag on the attribute, auto-attribute, and
override elements; has 3 options: none, safe, and any; the comments in
the XSD file describe what each of these do; the important thing to
know is that none is the default meaning no html is allowed; if html
is needed use safe and look at the antisamy-esapi.xml file to see
policy details; in extreme trust cases use any where any html is
allowed; note that many services need updating which should allow at
least safe html, and it may take some time to discover all of those
and get them handled; please send in issues and requests for service
attributes that should allow safe html
On Jan 23, 2009, at 3:41 AM, David E Jones wrote:
Hello all.
I'm actually a little surprised we're still where we are on this, so
I'm putting some time into this... understanding that it will annoy
as many people as it pleases (at first anyway...).
In order to address various XSS and XSS-like security threats, I'd
like to get some real and comprehensive stuff in place. Right now
there are super-easy attacks that can be done, like putting
JavaScript in a field during checkout that gets executed when a CSR
(or anyone using the Order Manager) looks at the order, or someone
looks at it in the Party Manager or wherever. That script can grab
the session id and send it to a URL for session hijacking, or it can
directly perform some action like marking the order as paid offline
or creating a new admin account or changing the users password or
whatever. The script could do anything the poor back-end user has
access to do, and that's just an example.
The best issues on this are:
https://issues.apache.org/jira/browse/OFBIZ-1959 (newer one, good
review of OFBiz security and applicable comments, good tips to
resolve)
https://issues.apache.org/jira/browse/OFBIZ-260 (the old/original
one, including my silly comment on it)
We have some simple code that does escaping for HTML chars, but it's
not really used anywhere. Anyway, I think we need something more
robust and comprehensive, especially given the fun ways of getting
around filters and other things presented here:
http://ha.ckers.org/xss.html
What I'd like to do is add the OWASP ESAPI library, which is BSD
licensed. There is a nice presentation about it as well here:
http://code.google.com/p/owasp-esapi-java/
and JavaDocs here:
http://owasp-esapi-java.googlecode.com/svn/trunk_doc/index.html
======================================
So, there's a tool, now how/where to use it in OFBiz...
I think this will require a fair bit of work, and I know I'll miss
things that are important in this first pass, but we can do some
things to take care of the more obvious problems:
1. validate input: consider not allowing HTML in any field by
default, and require an attribute set on service attributes or
possibly even entity fields to say that restricted/safe HTML is
allowed, or any HTML is allowed; this will break some things that
actually need HTML initially, but fixing the broken things once
found will be really easy
2. encode output: just in case HTML chars do get in somehow, we want
to encode them so they are displayed literally and not interpreted
as HTML by the browser; this will help avoid problems with messing
up formatting when HTML is present, as well as helping with this
security problem; this is easy to do in the various widgets (Screen,
Form, Tree, Menu), and is tougher in FTL files if we want it encoded
by default instead of manually using ?html on every field we want
encoded, and I'd rather use the ESAPI encoder than the FTL one too;
since much of this data is displayed right out of GenericValue
objects, one possible solution is to change the GenericValue.get
methods to do this encoding, and add a new get method that will not
do encoding; this would handle the situations where the GenericValue
is treated like a Map; this may also cause some crazy stuff to
happen in places where gets are used in services and such and not in
the UI... but I'm still thinking that through and am not sure if it
will be a problem... it is kind of using bomb to swat a fly so
collateral damage is likely
3. consider adding a token that is required for all requests in a
session except the first one, use a constantly changing token, and
have it added by the URL writing stuff based on a value in the
session; this would change on every request, which is a pain because
it means that any page in someone's browser other than the most
recently rendered one would not work (a problem we have with the
externalLoginKey stuff) unless we keep a certain number of the most
recent tokens in the session and allow any of the last 10 or 20 or
something to be used
4. related to #3, and relevant whether or not we do #3, add a unique
token to all rendered forms and require that when processing the
form input; if we only allow the tokens to be used once this also
fits the common pattern used for eliminating accidental multiple
submissions of the same form; this could be done easily with the
Form Widget and the ServiceEventHandler (or perhaps all of the event
handlers...), and more manually supported in other places like FTL
forms; this would require some configuration, and again the annoying
part is to cover as much as possible we would want this on by
default which may cause problems for some things which would then
need to changed to support it or disable it for that particular form
and/or event
====================================
I'm really interested in hearing what others have to say about
these. Personally I've avoided most of these types of things because
they always tend to cause a dozen problems for every problem they
solve. I've mentioned some concerns, but there are many more. Some
issues may just make the application less usable because of
restrictions on being able to do things like use the back button
(IMO supporting that is a critical part of any web app that is worth
anything) or having a bunch of false positives for security errors
because of some funny scenario that was not anticipated (and this
isn't an if thing, it's a when and how often thing).
-David
