I'm using SQL views quite extensively to run custom SQL in GeoServer, but
I've now reached a point where my developers are raising bug reports to
modify the validation regular expression to allow identifiers that contain
quotes.
At the moment I can't do this, or I could only do it by having developers
escape quotes themselves in advance and come up with a validation
expression to allow this.
I'd like to propose an enhancement to SQL views to allow the option of SQL
escaping a string that has already passed regular expression validation
before running the SQL.
I had intended to use commons-lang escapeSql() to do this but it only
escapes quotes and is gone in the latest release, so I was thingking of
writing a utility class to do this escaping myself since I can't find a
prebuilt class that does this in Java anywhere - the advice is just to use
bind parameters which won't work for things like table names and whole
lists for WHERE..IN.
If I open a feature enhancement request on Jira and create a pull request
with this functionality as an optional extra, does that sound alright to
everyone?
I’d like to propose an enhancement to SQL views to allow the option of SQL
escaping a string that has already passed regular expression validation
before running the SQL
I think this would just be in the geoserver code. Idea is to apply the escaping to the parameters if they pass regexp validation so it would be right next to that code where ever it is at the moment
I’d like to propose an enhancement to SQL views to allow the option of SQL
escaping a string that has already passed regular expression validation
before running the SQL
–
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
On Fri, Apr 12, 2013 at 9:40 AM, <geoff@anonymised.com> wrote:
Hi List,
I'm using SQL views quite extensively to run custom SQL in GeoServer, but
I've now reached a point where my developers are raising bug reports to
modify the validation regular expression to allow identifiers that contain
quotes.
At the moment I can't do this, or I could only do it by having developers
escape quotes themselves in advance and come up with a validation
expression to allow this.
I'd like to propose an enhancement to SQL views to allow the option of SQL
escaping a string that has already passed regular expression validation
before running the SQL.
I had intended to use commons-lang escapeSql() to do this but it only
escapes quotes and is gone in the latest release, so I was thingking of
writing a utility class to do this escaping myself since I can't find a
prebuilt class that does this in Java anywhere - the advice is just to use
bind parameters which won't work for things like table names and whole
lists for WHERE..IN.
If I open a feature enhancement request on Jira and create a pull request
with this functionality as an optional extra, does that sound alright to
everyone?
This sounds like a backwards incompatible change, I believe it should
be a separate flag?
By the way, the regex validation is done in GeoTools as well
Intention is to add a checkbox to the UI to activate the option. If enabled, all parameters that pass regexp validation will then also be escaped. This lets user’s pass parameters such as “o’shea” without being able to accidentally shoot themselves in the head if the regexp they are using to allow such strings lets in more then they were expecting. I think by forcing escaping of quotes and backslashes, we eliminate the most injection attacks that may have been accidentally enabled by regexps that are too simplistic or lax.
Perhaps this option could be active by default for new layers and disabled on existing ones?
I would be doing this extra step after the validation has been done. Haven’t had a good look yet but assumed it would be all in GeoServer, if not then I’ll add the corresponding code to GeoTools as well.
Cheers,
Geoff
···
I’d like to propose an enhancement to SQL views to allow the option of SQL
escaping a string that has already passed regular expression validation
before running the SQL.
This sounds like a backwards incompatible change, I believe it should
be a separate flag?
By the way, the regex validation is done in GeoTools as well
I'd like to propose an enhancement to SQL views to allow the option
of SQL
escaping a string that has already passed regular expression validation
before running the SQL.
This sounds like a backwards incompatible change, I believe it should
be a separate flag?
I've added it as an checkbox in the GeoServer GUI on the edit sql view page and as an option (default ON) in GeoTools
By the way, the regex validation is done in GeoTools as well
Your right Andrea- it's in the VirtualTable class which is where I added the optional SQL escaping step. I had it in my mind to do it all in GeoServer by escaping the variables before passing them to GeoTools but this would have to be done for each service (WFS, WMS, etc) and also makes it difficult to access the feature source to see if the option is enabled or not...
Let me know if this needs any more work. Cheers,
Geoff
This sounds like a backwards incompatible change, I believe it should
be a separate flag?
I've added it as an checkbox in the GeoServer GUI on the edit sql view
page and as an option (default ON) in GeoTools
By the way, the regex validation is done in GeoTools as well
Your right Andrea- it's in the VirtualTable class which is where I added
the optional SQL escaping step. I had it in my mind to do it all in
GeoServer by escaping the variables before passing them to GeoTools but
this would have to be done for each service (WFS, WMS, etc) and also makes
it difficult to access the feature source to see if the option is enabled
or not...
Let me know if this needs any more work. Cheers,
Thanks, I'll have a look as soon as possible. Given that at work I'm
buried , and this weekend I have to help doing the 2.3.1 release (since no
other developer stepped in)... it's likely going to be delayed to the
weekend after it
Thanks, I'll have a look as soon as possible. Given that at work I'm
buried , and this weekend I have to help doing the 2.3.1 release (since no
other developer stepped in)... it's likely going to be delayed to the
weekend after it
No worries Andrea, and thanks for the quick response! I'm just about to backport this to 2.3x along with the viewParams in post requests stuff if anyone needs it right away...