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
