Thread: Sql injection attacks
Hi folks, I'm new to Postgres and trying to get up to speed on the security issues. There seems to be remarkably little Postgres specific stuff on preventing SQL injection attacks. Most of the online literature is on MS SQL Server. There, the consensus seems to be that the range of potential attacks is so wide that attempting to spot attack signatures in posted data is a doomed enterprise, and that the safest general approach for any dynamically built query is to execute it as a stored procedure. In SQL Server, this reportedly works because the syntax of the query is pre-compiled, and the variables passed in are treated strictly as data and cannot alter the syntax. So any malicious use of "AND", "UNION", ";" etc in submitted data will fail. Can anyone confirm that this would also apply to Postgres Query Language (SQL) functions? The effectiveness of moving the queries into the database against SQL injection attack would seem to depend on the query engine internals. Will using the SQL functions provide the robust protection I am looking for? ------------------ Geoff Caplan Vario Software Ltd (+44) 121-515 1154
Geoff Caplan wrote: > I'm new to Postgres and trying to get up to speed on the security > issues. There seems to be remarkably little Postgres specific stuff > on preventing SQL injection attacks. If you use prepared statements (the details of which vary by programming language), you should be quite safe. > Most of the online literature is on MS SQL Server. There, the > consensus seems to be that the range of potential attacks is so wide > that attempting to spot attack signatures in posted data is a doomed > enterprise, and that the safest general approach for any dynamically > built query is to execute it as a stored procedure. That won't necessarily help you on PostgreSQL, because in stored procedures you can paste together queries from user-provided strings. -- Peter Eisentraut http://developer.postgresql.org/~petere/
Geoff Caplan <geoff@variosoft.com> wrote: > Hi folks, > > I'm new to Postgres and trying to get up to speed on the security > issues. There seems to be remarkably little Postgres specific stuff on > preventing SQL injection attacks. > > Most of the online literature is on MS SQL Server. There, the > consensus seems to be that the range of potential attacks is so wide > that attempting to spot attack signatures in posted data is a doomed > enterprise, and that the safest general approach for any dynamically > built query is to execute it as a stored procedure. Huh? To protect yourself from SQL injections, just pass all your data through PQescapeString() (or the appropriate function for the language you're developing in). That will escape any character that could be used to initiate an SQL injection, thus foiling the attempt. That "consensus" you speak of doesn't make much sense to me. Besides, who cares about "detecting" the SQL injection, as long as it's unable to work. > In SQL Server, this reportedly works because the syntax of the query > is pre-compiled, and the variables passed in are treated strictly as > data and cannot alter the syntax. So any malicious use of "AND", > "UNION", ";" etc in submitted data will fail. > > Can anyone confirm that this would also apply to Postgres Query > Language (SQL) functions? The effectiveness of moving the queries into > the database against SQL injection attack would seem to depend on the > query engine internals. Will using the SQL functions provide the > robust protection I am looking for? No, and I can't see how it will protect in MS SQL server either? They'll just learn that they have to escape out of the command that calls the stored function. -- Bill Moran Potential Technologies http://www.potentialtech.com
Hi folks, Peter Eisentraut wrote: PE> If you use prepared statements (the details of which vary by >> PE> programming language), you should be quite safe. Peter - thanks for the suggestion. You are right: a poorly designed function might simply concatenate the injected code - I hadn't really thought it through. The key seems to be to treat the unsafe string as a value so it can't leak out into the statement, and a parameterised prepared statement would do this effectively, as you suggest. Very elegant... Bill Moran wrote: BM> To protect yourself from SQL injections, just pass all your data through BM> PQescapeString() I'm no expert, but the papers I have been reading suggest that the usual hygene advice such as don't display DB error messages and escape unsafe strings doesn't cover all types of attack. See, for example, this: http://www.net-security.org/article.php?id=571 But so far as I can see, Peter's suggestion should provide a workable robust solution. So thanks again! ------------------ Geoff Caplan Vario Software Ltd (+44) 121-515 1154
Geoff Caplan <geoff@variosoft.com> wrote: > BM> To protect yourself from SQL injections, just pass all your data through > BM> PQescapeString() > > I'm no expert, but the papers I have been reading suggest that the > usual hygene advice such as don't display DB error messages and escape > unsafe strings doesn't cover all types of attack. See, for example, > this: > > http://www.net-security.org/article.php?id=571 > > But so far as I can see, Peter's suggestion should provide a workable > robust solution. So thanks again! Hope that works for you. I still think you're ignoring basic data validation. Simply put: 1) If the untrusted value is a string, using a proper escape sequence should make it safe. 2) If the untrusted value is not a string, then it should be tested for proper value (i.e. if it should be a number, it should be ensured that it _is_ a number, and nothing else) invalid values should trigger an error. I don't see how storing the SQL in some different location is the correct way to fix anything? Besides, the suggestions made in that paper only work with ASP and JDBC ... how do you accomplish the same thing in PHP, for example? Just my opinion. Take it or leave it as you see fit. -- Bill Moran Potential Technologies http://www.potentialtech.com
Bill Moran <wmoran@potentialtech.com> wrote: > [snip] > > Simply put: > 1) If the untrusted value is a string, using a proper escape sequence should > make it safe. > 2) If the untrusted value is not a string, then it should be tested for > proper value (i.e. if it should be a number, it should be ensured that > it _is_ a number, and nothing else) invalid values should trigger an > error. > > I don't see how storing the SQL in some different location is the correct > way to fix anything? [snip] > I agree with Bill. Years ago (more years than I care to recall) I read a book on structured systems design (IIRC) that advised one should condition/convert data as early as possible in the process, throughout the design. Amongst the advantages cited for this tactic was that then you would know, everywhere else in the system, that you were dealing only with conditioned data. That practice, taken to heart relatively early in my career, has always stood me in good stead. Thus I recommend to others the same approach. In short: Any data coming from an untrusted source should always be de-fanged as early as possible. Jim
Bill Moran <wmoran@potentialtech.com> writes:
> Geoff Caplan <geoff@variosoft.com> wrote:
>
> > Hi folks,
> >
> > I'm new to Postgres and trying to get up to speed on the security
> > issues. There seems to be remarkably little Postgres specific stuff on
> > preventing SQL injection attacks.
> >
> > Most of the online literature is on MS SQL Server. There, the
> > consensus seems to be that the range of potential attacks is so wide
> > that attempting to spot attack signatures in posted data is a doomed
> > enterprise, and that the safest general approach for any dynamically
> > built query is to execute it as a stored procedure.
>
> Huh?
>
> To protect yourself from SQL injections, just pass all your data through
> PQescapeString()
Or better yet don't mix your data with your code.
Any the literature that suggests interpolating your data into your SQL queries
using some escaping mechanism is in my humble opinion, leading you down the
garden path. It's the wrong way to think about things.
You should never ever write code that mixes data with executable code. Doing
so is just asking for trouble. Even if you know about PQEscapeString, it's
hard to verify that PQEscapeString has been called in every single place where
it's needed. One day you'll miss one place and all that effort becomes futile.
Better to just never mix the two. Let the driver handle marshalling the data
and transporting it to the database. All good driver APIs have an interface
that allows you to ship the data as separate parameters.
Something like (in Perl)
$sth = $dbh->prepare('select * from foo where a=?');
$sth->execute($dangerous_data);
or (in PHP)
$row = $db->getone('select * from foo where a=?', array($dangerous_data));
If you get into the habit of doing things this way normally, never just
interpolating variables into your sql code, then the rare instance when you do
have to interpolate something will stand out like a sore thumb. And it should
be easy to see whether that something is safely coming from static data in the
code or is dangerously coming from outside data from the network.
Note that not all database engines and drivers will actually be able to avoid
interpolating the data into the SQL eventually. Postgres as of 7.4 is capable
of it but even for Postgres not all the drivers have been updated. But that's
mostly irrelevant, at least make it the responsibility of the driver to do the
interpolating, it's more likely to get it right and whatever it does, it will
at least be 100% consistent about it.
But mixing the data with the code, even if you use PQEscapeString is just a
bad programming practice.
--
greg
Bill Moran wrote:
>
>
>Simply put:
>1) If the untrusted value is a string, using a proper escape sequence
should
> make it safe.
>
>
in pgsql (and mysql) you can escape almost everything.
update table set a = '5' is corrent, even is column a is integer type.
You can't escape the null value.
Mage
> Most of the online literature is on MS SQL Server. There, the
> consensus seems to be that the range of potential attacks is
> so wide that attempting to spot attack signatures in posted
> data is a doomed enterprise, and that the safest general
> approach for any dynamically built query is to execute it as
> a stored procedure.
This recommendation is one of the main reasons there are *soooo* many
SQL-injection vulnerable sites built on the ASP/SQLServer combination by
people that *think* they are safe. It used to be in the MS
documentation, but it's supposed to be all gone now. But it's still in a
lot of other guides out there.
Using a stored procedure is just as unsafe as using a direct SQL
statement, *unless* you use the parametrised (eh, is it really called
that? Wel..) format. And the other way around, using direct SQL is just
as safe *iff* you use parametetrised format.
"SELECT * FROM foo WHERE bar=" & request("bar")
is no more dangerous than
"sp_foo " & request("bar")
If you use the parametrised formats, e.g.
"SELECT * FROM foo WHERE bar=@bar"
or just "sp_foo" in the case of the stored procedure (the driver will
pick up parameter names automatically)
Then it's the responsibility of the driver to handle any escaping
needed. And AFAIK it does a good job of it.
Similar methods are of course available in pretty much all other DB
interfaces, depending on language/framework.
> In SQL Server, this reportedly works because the syntax of
> the query is pre-compiled, and the variables passed in are
> treated strictly as data and cannot alter the syntax. So any
> malicious use of "AND", "UNION", ";" etc in submitted data will fail.
Yes, this will fail. However, injecting something like:
"sp_foo 'bar' ; DROP TABLE foo"
is still very possible.
> Can anyone confirm that this would also apply to Postgres
> Query Language (SQL) functions? The effectiveness of moving
> the queries into the database against SQL injection attack
> would seem to depend on the query engine internals. Will
> using the SQL functions provide the robust protection I am
> looking for?
No. Parametrised queries will.
There are other reasons for moving the querys to the backend (isolation
and performance depending on your situation), but it won't provide the
protection you're asking for here.
//Magnus
Hi folks
Seems we have two schools of thought:
1) The validation/escaping approach, supported by Bill and Jim
2) The "don't mix data with code" approach supported by Peter and
Greg.
As I learn more about the issues, I am increasingly veering towards
the second approach.
Obviously, proper validation is a given for all kinds of reasons. But
the problem with validation/escaping as the primary defense against
injection seems to be that simply escaping would not catch every type
of insertion via strings.
If you are just saving the untrusted string as a value, then the
attacker needs to use a quote or comment to escape out of the value
and alter the query syntax. This is relatively easy to counter, as
Bill suggests.
But in web work, you are often using GET/POST data directly in your
SQL clauses, so the untrusted data is part of the query syntax and not
just a value.
In that scenario, if you just concatenate your untrusted data into
your query, it is relatively easy for the attacker to sneak in
AND/OR/UNION type clauses without using quotes. This means that you
have to check strings for attack signatures as well, which is not
trivial if you are going to avoid false positives. And it means that
every single untrusted string has to be screened by someone skilled at
analysing the potential threats and writing the right Regexes. And as
Greg point out, you only have to screw up once and you have opened up
your whole system. Plus, as new attacks are developed, you have to
audit all your code to counter them. This surely can't be the way to
go.
Bill wrote:
BM> I don't see how storing the SQL in some different location is the
BM> correct way to fix anything?
That was my original suggestion, and I agree it was naive. But I don't
think that is what Peter is suggesting. I think the idea is that
parameterised prepared statements compile the SQL statement before the
user input is added via the parameters, making it impossible for user
input to modify the actual SQL statement.
From the practical point of view, it surely seems easier to adopt a
particular style of programming ("any query using untrusted data MUST
be a parameterised prepared statement") than to analyse each and every
untrusted parameter for attack signatures? Or am I missing something
here?
------------------
Geoff Caplan
Vario Software
(+44) 121-515 1154
Magnus, Your posting arrived just after I posted my attempt at a summary... With the help of the list, I had already got to the stage that parameterised queries are the way to go. Your post helps confirm that. Now I need to understand the implementation details. Clearly, one option is the PREPARE/EXECUTE statements. But I am not sure I follow you when you say: MH> If you use the parametrised formats, e.g. MH> "SELECT * FROM foo WHERE bar=@bar" MH> or just "sp_foo" in the case of the stored procedure (the driver will MH> pick up parameter names automatically) Are you saying that "@bar" is part of the Postgres query syntax? If it is, I can't find it in the 7.4 docs. What do you mean by "the parameterised formats"? I would appreciate a clarification and the URL of the relevant documentation. Thanks ------------------ Geoff Caplan Vario Software Ltd (+44) 121-515 1154
> MH> If you use the parametrised formats, e.g. > MH> "SELECT * FROM foo WHERE bar=@bar" > MH> or just "sp_foo" in the case of the stored procedure (the driver > MH> will pick up parameter names automatically) > > Are you saying that "@bar" is part of the Postgres query > syntax? If it is, I can't find it in the 7.4 docs. What do > you mean by "the parameterised formats"? I would appreciate a > clarification and the URL of the relevant documentation. Sorry, this is the ADO syntax used in ASP - I was referring to the incorrect claims about MSSQL you had found. It definitly applies to MSSQL on ASP, not 100% sure about pgsql on ASP. Another common syntax is the "?" syntax, e.g. "SELECT * FROM foo EBRE bar=?", that's used in for example DBI. So it depends on what interface you are using to connect to the server. If it's straight libpq, you should look at PQexecParams() or PQexecPrepared(), see http://www.postgresql.org/docs/7.4/static/libpq-exec.html. //Magnus
On Mon, Jul 26, 2004 at 08:08:35AM +0200 I heard the voice of Mage, and lo! it spake thus: > Bill Moran wrote: > > > >Simply put: > >1) If the untrusted value is a string, using a proper escape > > sequence should make it safe. > > in pgsql (and mysql) you can escape almost everything. > > update table set a = '5' is corrent, even is column a is integer type. > You can't escape the null value. Which, IMO, is a great thing; I studiously trained myself to use the escaping functions on every value I ever use in a query. If you escape everything unconditionally, without worrying about what type the column is, there's a lot less chance for mistakes and oversights. -- Matthew Fuller (MF4839) | fullermd@over-yonder.net Systems/Network Administrator | http://www.over-yonder.net/~fullermd/ "The only reason I'm burning my candle at both ends, is because I haven't figured out how to light the middle yet"
Geoff Caplan <geoff@variosoft.com> writes: > But in web work, you are often using GET/POST data directly in your > SQL clauses, so the untrusted data is part of the query syntax and not > just a value. Can you give an example of this that isn't also an example of obviously bad application design? -Doug -- Let us cross over the river, and rest under the shade of the trees. --T. J. Jackson, 1863
Doug, DM> Geoff Caplan <geoff@variosoft.com> writes: >> But in web work, you are often using GET/POST data directly in your >> SQL clauses, so the untrusted data is part of the query syntax and not >> just a value. DM> Can you give an example of this that isn't also an example of DM> obviously bad application design? I'm no expert to put it mildly, but if you Google for "SQL Injection Attack" you'll find a lot of papers by security agencies and consultancies. You could start with these: www.nextgenss.com/papers/advanced_sql_injection.pdf http://www.net-security.org/article.php?id=142 They are SQL Server oriented, but many of the issues would apply to Postgres. ------------------ Geoff Caplan Vario Software Ltd (+44) 121-515 1154
Geoff Caplan <geoff@variosoft.com> writes: > Doug, > > DM> Geoff Caplan <geoff@variosoft.com> writes: > >>> But in web work, you are often using GET/POST data directly in your >>> SQL clauses, so the untrusted data is part of the query syntax and not >>> just a value. > > DM> Can you give an example of this that isn't also an example of > DM> obviously bad application design? > > I'm no expert to put it mildly, but if you Google for "SQL Injection > Attack" you'll find a lot of papers by security agencies and > consultancies. You could start with these: That doesn't answer my question. :) If you're trusting the user (via GET or POST data) to hand you valid SQL fragments, even just column names, you Deserve To Lose. The only things that come in via GET or POST should be data values, and they should either be explicitly escaped, or used in prepared statements where the driver takes care of the escaping. -Doug -- Let us cross over the river, and rest under the shade of the trees. --T. J. Jackson, 1863
At 09:58 AM 7/26/2004 +0100, Geoff Caplan wrote: >Seems we have two schools of thought: > >1) The validation/escaping approach, supported by Bill and Jim > >2) The "don't mix data with code" approach supported by Peter and >Greg. > >As I learn more about the issues, I am increasingly veering towards >the second approach. They are not really different schools of thought. My suggestion: 0) Make it much easier to the right thing than the wrong thing. Prepared statements help. You may still need to make it simpler for the programmers. 1) validate/filter/escape all input to your program so that your program (NOT other programs[1]) can deal with it. 2) validate/filter/escape output from your program to each destination accordingly so that each destination can deal with it and treat it correctly. 3) Assume by default input could be from hostile sources, unless provable otherwise. [1] Do not combine filters, doing so often corrupts data. Boneheaded thinking like PHP's magic quotes create the silly multiple backslashed single quotes and similar stuff. Also for PostgreSQL at least do not forget that _ and % are wildcards for LIKE and similar queries and these wildcards are probably NOT escaped whether explicitly by postgresql's builtin escaping functions or automatically by the parameterizing methods. When I last checked you have to escape them yourself if you want to. Though insertion of wildcards is not exactly SQL injection they often cause applications to behave differently from design. A wildcard in a LIKE query could cause a full table scan, and thus: 1) Use up resources (CPU, memory, disk/network bandwidth). Possibly more easily allow a Denial of Service attack. 2) Indicate to the attacker how many rows there are (e.g. how many customers a particular CA had at a particular moment. Polling regularly indicates how many customers added, or whether _your_ row was added successfully (*grin*) ). >But in web work, you are often using GET/POST data directly in your >SQL clauses, so the untrusted data is part of the query syntax and not >just a value. Don't do that. If you need to do that, you are doing things wrong. Very wrong. Link.
Geoff Caplan <geoff@variosoft.com> writes:
> Obviously, proper validation is a given for all kinds of reasons. But
> the problem with validation/escaping as the primary defense against
> injection seems to be that simply escaping would not catch every type
> of insertion via strings.
I think you misunderstood. Escaping is perfectly safe (given a correct
escaping function) if it's used on *every* untrustworthy input string.
The argument for the "keep data separate from code" approach is
essentially just that it's easier to be sure you haven't forgotten
anyplace where you need to escape.
regards, tom lane
Hi folks Doug wrote: >> If you're trusting the user (via GET or POST data) to hand you valid >> SQL fragments, even just column names, you Deserve To Lose. The only >> things that come in via GET or POST should be data values, and they >> should either be explicitly escaped, or used in prepared statements >> where the driver takes care of the escaping. I don't think we are disagreeing here - seems like sound advice to me. I am just feeling my way into this database game, so perhaps I am not expressing myself clearly... Lincoln wrote: >> They are not really different schools of thought. >> My suggestion: >> 1) validate/filter/escape all input to your program so that your program >> (NOT other programs) can deal with it. That makes sense. What I am currently planning is to: a) Restrict application urls to a small set of safe characters, excluding spaces. The front-controller will run a safety check on the url and abort the request if it looks odd. This should make it impossible to inject into a GET variable. All cookies will carry a digest with secret key so they can't be spoofed. That only leaves posted and uploaded data to worry about. b) Only write raw SQL for internal queries with static data. All other queries will be either parameterised prepared queries, or constructed via an API that will quote every value. The info from another poster that it is OK to quote numbers makes this simple. >> Also for PostgreSQL at least do not forget that _ and % are wildcards for >> LIKE and similar queries and these wildcards are probably NOT escaped >> whether explicitly by postgresql's builtin escaping functions or >> automatically by the parameterizing methods. When I last checked you have >> to escape them yourself if you want to. >> Though insertion of wildcards is not exactly SQL injection they >> often cause applications to behave differently from design. Very useful: I wouldn't have thought of this... >>But in web work, you are often using GET/POST data directly in your >>SQL clauses, so the untrusted data is part of the query syntax and not >>just a value. >> Don't do that. If you need to do that, you are doing things wrong. Very wrong. Please disregard that statement - I wasn't thinking clearly. What I had in mind was that something like a product id in a url might find its way into, say, a WHERE clause. What I overlooked is that it is still, of course, being used as a value. My inexperience is showing, I'm afraid! ------------------ Geoff Caplan Vario Software Ltd (+44) 121-515 1154
Doug McNaught wrote:<br /><blockquote cite="mid877jsq7seo.fsf@asmodeus.mcnaught.org" type="cite"><pre wrap="">Geoff Caplan<a class="moz-txt-link-rfc2396E" href="mailto:geoff@variosoft.com"><geoff@variosoft.com></a> writes: </pre><blockquote type="cite"><pre wrap="">Doug, DM> Geoff Caplan <a class="moz-txt-link-rfc2396E" href="mailto:geoff@variosoft.com"><geoff@variosoft.com></a> writes: </pre><blockquote type="cite"><blockquote type="cite"><pre wrap="">But in web work, you are often using GET/POST datadirectly in your SQL clauses, so the untrusted data is part of the query syntax and not just a value. </pre></blockquote></blockquote><pre wrap="">DM> Can you give an example of this that isn't also anexample of DM> obviously bad application design? I'm no expert to put it mildly, but if you Google for "SQL Injection Attack" you'll find a lot of papers by security agencies and consultancies. You could start with these: </pre></blockquote><pre wrap=""> That doesn't answer my question. :) If you're trusting the user (via GET or POST data) to hand you valid SQL fragments, even just column names, you Deserve To Lose. The only things that come in via GET or POST should be data values, and they should either be explicitly escaped, or used in prepared statements where the driver takes care of the escaping. -Doug </pre></blockquote> Or, if your POST/GET data is necessary to alter your SQL statement, then make it conditional, butnever accept raw SQL from the user. It's easy enough to check for certain fields (such as a checkbox for a boolean ANDor OR in your where clause). But be sure that the checkbox never sends in SQL code, it should just send in a 1 if checked. Then in your code, you check for that value and you manually program the 2 alternative versions of the SQL statement. It's more work in design, but it's easier in the long run.<br /><br /> As for escaping the data still being likelyto cause problems, if you escape all of the quotes in your data, then the data can never be outside of one of yourquoted columns of data. If the user sends in the data "valid data' SQL code here", and you escape, that entire stringbecause just data, because it becomes "valid data\' SQL code here". As a result, it will never be allowed outsideof the column data (ie, the entire thing will be treated as data). If the field is a non-character field, you cansimply strip out all quotes, and any "data" that is invalid will be rejected by the PostgreSQL engine. For example, ifa date field is what the user is trying to compromise, and you remove the quotes, the system will just give an error sayinginvalid date format. This is, of course, if you don't want to do software validation (i.e. basic data reasonabilitychecks).<br /><br /> Personally, I don't trust the users to give me valid SQL fragments, so I only treat inbounddata as data. Users are dangerous, and users with a little knowledge are more dangerous, so I don't want my usersto have a need or desire to learn SQL.<br /><pre class="moz-signature" cols="72">-- Thanks, Laura Vance Systems Engineer Winfree Academy Charter Schools, Data-Business Office 1711 W. Irving Blvd. Ste 310 Irving, Tx 75061 Web: <a class="moz-txt-link-abbreviated" href="http://www.winfreeacademy.com">www.winfreeacademy.com</a> </pre>
Greg Stark <gsstark@mit.edu> wrote:
>
> Bill Moran <wmoran@potentialtech.com> writes:
>
> > Geoff Caplan <geoff@variosoft.com> wrote:
> >
> > > Hi folks,
> > >
> > > I'm new to Postgres and trying to get up to speed on the security
> > > issues. There seems to be remarkably little Postgres specific stuff on
> > > preventing SQL injection attacks.
> > >
> > > Most of the online literature is on MS SQL Server. There, the
> > > consensus seems to be that the range of potential attacks is so wide
> > > that attempting to spot attack signatures in posted data is a doomed
> > > enterprise, and that the safest general approach for any dynamically
> > > built query is to execute it as a stored procedure.
> >
> > Huh?
> >
> > To protect yourself from SQL injections, just pass all your data through
> > PQescapeString()
>
> Or better yet don't mix your data with your code.
>
> Any the literature that suggests interpolating your data into your SQL queries
> using some escaping mechanism is in my humble opinion, leading you down the
> garden path. It's the wrong way to think about things.
>
> You should never ever write code that mixes data with executable code. Doing
> so is just asking for trouble. Even if you know about PQEscapeString, it's
> hard to verify that PQEscapeString has been called in every single place where
> it's needed. One day you'll miss one place and all that effort becomes futile.
>
> Better to just never mix the two. Let the driver handle marshalling the data
> and transporting it to the database. All good driver APIs have an interface
> that allows you to ship the data as separate parameters.
>
> Something like (in Perl)
>
> $sth = $dbh->prepare('select * from foo where a=?');
> $sth->execute($dangerous_data);
>
> or (in PHP)
>
> $row = $db->getone('select * from foo where a=?', array($dangerous_data));
I've hit a lot of problems with these type of interfaces making it very
difficult to execute complex queries. But it may just be my unfamiliarity
with such coding conventions. I avoid them because they're difficult, but
they're difficult because I avoid them.
However, how do you suggest that rule of thumb be done when working in C?
--
Bill Moran
Potential Technologies
http://www.potentialtech.com
Bill Moran <wmoran@potentialtech.com> writes: > I've hit a lot of problems with these type of interfaces making it very > difficult to execute complex queries. But it may just be my unfamiliarity > with such coding conventions. I avoid them because they're difficult, but > they're difficult because I avoid them. Occasionally I find I have to interpolate something like $join_type which I either set to "OUTER" or "INNER" in my own code just prior to the query. That's obviously safe since the data comes from static data in my code, not network data. It also only results in two different plans, not any number of plans depending on what data the user provides. I also often have an idiom like: WHERE (?=0 OR data like '%'||?||'%') AND (?=0 OR size > ?) And call it with parameters like $search ne '', $search, $min_size > 0, $min_size This avoids having to construct different queries with different plans for all the different combination of search constraints. This matters even more when you start to worry about reducing planning time by caching all your prepared statements. Perl DBI even provides a mechanism to do this automatically. But to use it reliably you have to be sure you aren't going to generate an infinite number of SQL statements with hard coded parameters in them. > However, how do you suggest that rule of thumb be done when working in C? http://www.postgresql.org/docs/7.4/interactive/libpq-exec.html#AEN20466 The primary advantage of PQexecParams over PQexec is that parameter values may be separated from the command string, thus avoiding the need for tedious and error-prone quoting and escaping. I would go so far as to suggest deprecating and eventually removing PQExec entirely in favour of PQExecParams. There's no need for the former and having people see the params parameter all the time might give them the idea that it would be wise to use it. Incidentally, you should be able to prepare queries and execute them later like the DBI and PHP interfaces, but there's an odd comment in the docs: Presently, prepared statements for use with PQexecPrepared must be set up by executing an SQL PREPARE command, which is typically sent with PQexec (though any of libpq's query-submission functions may be used). A lower-level interface for preparing statements may be offered in a future release. I don't think this is true any more. I think the low level protocol exists now. It's possible the libpq method doesn't exist yet though. -- greg
At 04:33 PM 7/26/2004 +0100, Geoff Caplan wrote:
>Lincoln wrote:
>
> >> They are not really different schools of thought.
>
> >> My suggestion:
>
> >> 1) validate/filter/escape all input to your program so that your program
> >> (NOT other programs) can deal with it.
>
>That makes sense. What I am currently planning is to:
I would like to emphasize that my suggestion is items 0 to 3 as a whole.
They were not really meant as independent items.
0) Make it much easier to the right thing than the wrong thing. Prepared
statements help. You may still need to make it simpler for the programmers.
1) validate/filter/escape all input to your program so that your program
(NOT other programs[1]) can deal with it.
2) validate/filter/escape output from your program to each destination
accordingly so that each destination can deal with it and treat it correctly.
3) Assume by default input could be from hostile sources, unless provable
otherwise.
---
More detailed example scenario:
First text is submitted to your program from a browser.
- You may wish to limit max submission size at the webserver level to X
megabytes (the size of the maximum submission - including attachment
uploads) so as to not use up too much memory in case of a malicious
submission.
- you may wish to limit the max text length at the program level to Y bytes
and do other sanity checking etc.
- You may wish to remove all ctrl characters except cr/lf from the submission.
- you may also wish to have a input parameter definining what sort of text
it is - preformatted text, normal text, restricted HTML.
Say the text is to be treated as nonpreformatted normal text so:
- You may wish to trim leading/trailing whitespace and squeeze extra
contiguous whitespace.
- You may wish to convert all cr to lfs then squeeze contiguous lfs to
single lfs.
Next: Text is submitted from your program to the database.
- you use a standardized function to insert the text into the database.
e.g. dbdo("Error to throw if fail","insert into test (data) values
(?)","$thetext");
or to put a row into a table.
dbput("Error to throw if
fail","tablename","$whereclause_to_try_to_match",$ColsAndDataToInsertOrUpdateIfExist);
Note: avoid situations that could allow things like this:
update tablea set data=3-? where a=1;
If the parameter is -1 and you are not careful the SQL could become this:
update tablea set data=3--1 where a=1;
-- is a comment! Wow what a great design huh?
I'm not sure what is the best way around this but you could try:
update tablea set data='3'-'-1'::integer where a=1;
Or ensuring there's a space between ? and all other characters.
Next: text is read from the database into your program
- limit text length to Y bytes and other sanity checking.
- you filter/process the text accordingly
Next: text is sent from your program to a web browser.
you read what sort of text it is.
Assuming the text is to be treated as normal text instead of HTML or
preformatted text.
- you filter and process the text, quoting & to & < to < and the
other usual suspects and convert all lf to <p>.
Result: The browser displays text as text and not some weird malicious
browser-subverting javascript. Phew :).
Error occurs: text is sent from your program to the logs.
You limit the text to a max length of A bytes.
You escape control characters and other junk so the logs and whatever you
use to view the logs don't do strange things.
You chop the text to pieces of max length B bytes
You send the text to the system logs and indicate dataloss/truncation if any.
Result: you can view the logs safely even on crappy terminals, and
backspaces etc don't result in stuff being hidden or rewritten.
Remember to make all that easy to do, otherwise it won't be done and one
day someone might abuse systems running your program!
Have a nice day,
Link.
p.s. sorry this is a bit off-topic. But tons of people seem to do things
the wrong way. Even the postgresql.org site had an SQL injection prob with
the prev survey form ;).
Greg Stark <gsstark@mit.edu> wrote: > > Bill Moran <wmoran@potentialtech.com> writes: > > > I've hit a lot of problems with these type of interfaces making it very > > difficult to execute complex queries. But it may just be my unfamiliarity > > with such coding conventions. I avoid them because they're difficult, but > > they're difficult because I avoid them. > > Occasionally I find I have to interpolate something like $join_type which I > either set to "OUTER" or "INNER" in my own code just prior to the query. > That's obviously safe since the data comes from static data in my code, not > network data. It also only results in two different plans, not any number of > plans depending on what data the user provides. > > I also often have an idiom like: > > WHERE (?=0 OR data like '%'||?||'%') > AND (?=0 OR size > ?) > > And call it with parameters like > > $search ne '', $search, > $min_size > 0, $min_size > > This avoids having to construct different queries with different plans for all > the different combination of search constraints. > > This matters even more when you start to worry about reducing planning time by > caching all your prepared statements. Perl DBI even provides a mechanism to do > this automatically. But to use it reliably you have to be sure you aren't > going to generate an infinite number of SQL statements with hard coded > parameters in them. Personal preference, I guess, but I still don't like it. I feel like I've got to push through 30 levels of abstraction just to get a query executed. I mean, if I want to move some things around a bit to get a better query plan, I don't want to have to figure out how the interface is going to juggle my arguments. > > However, how do you suggest that rule of thumb be done when working in C? > > http://www.postgresql.org/docs/7.4/interactive/libpq-exec.html#AEN20466 > > The primary advantage of PQexecParams over PQexec is that parameter values > may be separated from the command string, thus avoiding the need for tedious > and error-prone quoting and escaping. Yeah ... I don't know what I was thinking when I asked that question ... > I would go so far as to suggest deprecating and eventually removing PQExec > entirely in favour of PQExecParams. There's no need for the former and having > people see the params parameter all the time might give them the idea that it > would be wise to use it. Maybe, but it makes things more complicated the more dynamically the query is generated. I'm talking about variable numbers of WHERE clauses and dynamically chosen JOINs ans stuff ... where the SQL itself changes, as well as the # of parameters. For the most part, I agree with you on this point, although I wouldn't go so far as to suggest that PQExec should be removed. -- Bill Moran Potential Technologies http://www.potentialtech.com
In article <27702.1090854781@sss.pgh.pa.us>, Tom Lane <tgl@sss.pgh.pa.us> writes: > Geoff Caplan <geoff@variosoft.com> writes: >> Obviously, proper validation is a given for all kinds of reasons. But >> the problem with validation/escaping as the primary defense against >> injection seems to be that simply escaping would not catch every type >> of insertion via strings. > I think you misunderstood. Escaping is perfectly safe (given a correct > escaping function) if it's used on *every* untrustworthy input string. > The argument for the "keep data separate from code" approach is > essentially just that it's easier to be sure you haven't forgotten > anyplace where you need to escape. Exactly. As long as you escape everything, you're safe. The only thing to remember is that that you have to escape in both directions: whatever you get from your web page and want to put into the DB should be SQL-escaped, and whatever you get from the DB and want to display on a web page should be HMTL-escaped (including error messages from PostgreSQL).
Greg Stark <gsstark@mit.edu> writes:
> Incidentally, you should be able to prepare queries and execute them later
> like the DBI and PHP interfaces, but there's an odd comment in the docs:
> Presently, prepared statements for use with PQexecPrepared must be set up by
> executing an SQL PREPARE command, which is typically sent with PQexec
> (though any of libpq's query-submission functions may be used). A
> lower-level interface for preparing statements may be offered in a future
> release.
> I don't think this is true any more. I think the low level protocol exists
> now. It's possible the libpq method doesn't exist yet though.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
That's what the comment is trying to tell you: libpq does not currently
offer a way to use the V3-protocol Prepare message.
regards, tom lane
Jim Seymour wrote: > Bill Moran <wmoran@potentialtech.com> wrote: > > [snip] > > I agree with Bill. Years ago (more years than I care to recall) I read > a book on structured systems design (IIRC) that advised one should > condition/convert data as early as possible in the process, throughout > the design. Amongst the advantages cited for this tactic was that then > you would know, everywhere else in the system, that you were dealing > only with conditioned data. That practice, taken to heart relatively > early in my career, has always stood me in good stead. Thus I > recommend to others the same approach. > > In short: Any data coming from an untrusted source should always be > de-fanged as early as possible. > Sounds like reading up on perl's Taint feature would be beneficial here as well. They have the similar attitude that if it hasn't been specifically de-loused, then it probably has lice.
Geoff Caplan wrote: > Hi folks > > Seems we have two schools of thought: > > 1) The validation/escaping approach, supported by Bill and Jim > > 2) The "don't mix data with code" approach supported by Peter and > Greg. > > As I learn more about the issues, I am increasingly veering towards > the second approach. > Now I always assumed that the correct approach was always going to be D) ALL of the above. Furthermore, if you are really concerned about passing information through the URL, consider relating data in your database to sessions, cookies, and file caches to aliase all those fields you pass back and forth to a session ID or similar. The example of "...index.html?id=34" is sufficient for much of this though I doubt 'zine articles merit greater security than this.
Python has an interface like this :
params = { 'mystrfield': 'hello', 'myintfield': 5 }
cursor.execute( "SELECT myfield FROM mytable WHERE
mystrfield=%(foo)s AND myintfield=%(bar)d;" , params )
It has the following advantages :
- separation of sql from data
- named parameters
- no problem with order
- one parameter can be reused several times
- automatic escaping of strings
- automatic enforcement of int, float etc types with %d and %f (throws an
exception otherwise)
The only problem so far with Python's dbapi is that it does not
understand arrays so they have to be stringified first.
I created a table to hold some forum messages :
table messages (
id serial primary key,
parent_folder integer not null
references folders(id), --in another table
-- for replies, this stores the message which we reply to
reply_to_id integer null
references messages( id ),
thread_depth integer not null default 0
other fields :
message_title, message_text, author, etc.
);
I want to know which depth the messages in a thread are, simply to shift
them a little more to the right in the display, and I want to store this
in the "thread_depth" field so I don't have to compute it at each SELECT I
do.
My first thought was to use an "ON INSERT" rule which would change the
"thread_depth" of messages inserted with a not-null "reply_to_id" by
inserting an extra UPDATE after the INSERT. Problem was, the "NEW.id"
which got passed to me by postgresql contained something like :
(nextval('messages_id_seq')::text)::integer
Aside from the funky double cast, trying to read the value incremented
the sequence and the UPDATE failed because it tried to update a
non-existent row.
So, NEW contained unevaluated default values, BUT reading it evaluates
the values (of course) so it consumes an extra sequence number...
Is this normal ?
In the end I used a trigger which looks cleaner :
create or replace function vf.messages_set_depth()
returns trigger
as 'begin
if new.reply_to_id is not null then
new.thread_depth=1+(select thread_depth from messages where
id=new.reply_to_id limit 1);
end if;
return new;
end;'
language plpgsql;
create trigger messages_set_depth_trigger
before insert or update on messages
for each row execute procedure messages_set_depth();
And it works well this way. I don't intend to do mass message insertion
so the fact that the trigger is called on every insert does not disturb
me. However, the strange behaviour of the rule still bothers me.
Any comments ?
=?iso-8859-15?Q?Pierre-Fr=E9d=E9ric_Caillaud?= <lists@boutiquenumerique.com> writes:
> Is this normal ?
Yes. See the archives.
> In the end I used a trigger which looks cleaner :
Yup, triggers are the recommended way to solve this type of problem.
regards, tom lane
Thank you.
> Yes. See the archives.
I saw the "using triggers" advice but did not find info on why the rules
behaved that way. No problem, the trigger works.
At 11:13 PM 7/26/2004, Tom Lane wrote: >I think you misunderstood. Escaping is perfectly safe (given a correct >escaping function) if it's used on *every* untrustworthy input string. >The argument for the "keep data separate from code" approach is >essentially just that it's easier to be sure you haven't forgotten >anyplace where you need to escape. Just curious on what are the correct/official ways for dealing with the following scenarios: 1) Input string contains % and _ and would be used in a LIKE query that should not have user controllable wildcards. 2) Input string are numbers which can hold negative and positive values and could potentially be used in the following query: update tablea set a=10-$inputstring where key=1; When I tested at least one version of DBD::Pg doesn't appear to escape the inputstring when it's a number. e.g. $SQL="update tablea set a=10-? where key=1"; And $SQL is used in a prepared statement with $inputstring as a parameter. I found that when $inputstring contained a negative number, all rows in tablea are set to the same value. 3) Postgresql queries with select .... from ... where ... LIMIT $inputstring Postgresql 7.3.4 doesn't like LIMIT '1' Nor does it like LIMIT '1'::integer Haven't checked OFFSET. (sorry- I downgraded to 7.3.4 and haven't upgraded). Regards, Link.
> update tablea set a=10-$inputstring where key=1;
Add parenthesis:
update tablea set a=10-($inputstring) where key=1;
Thus you get :
update tablea set a=10-(-1) where key=1;
instead of :
update tablea set a=10--1 where key=1;
You'd have a problem because -- is the Comment Delimiter thus
update tablea set a=10--1 where key=1;
means :
update tablea set a=10;
which does update all rows.
I still think inputstring should be cast to int in YOUR code prior to be
sent to SQL, because then you can give a good error message to the user
instead of "SQL request failed".
Hi folks Very instructive thread. Thanks to everyone for the input, and especial thanks to Lincoln Yeoh for his detailed explanation of his approach: a standout post! Sorry for the silence - it's not that I'm unappreciative, just that I've been away from my desk. Tom Lane wrote: > I think you misunderstood. Escaping is perfectly safe (given a correct > escaping function) if it's used on *every* untrustworthy input string. > The argument for the "keep data separate from code" approach is > essentially just that it's easier to be sure you haven't forgotten > anyplace where you need to escape. You are right: I was misunderstanding the nature of the problem. I now appreciate that any effective approach must include both validation and escaping of all untrusted data used in a query, both when it is first passed into the application, and also when it is re-used from the data store. But I'd like to be sure I understand exactly what Tom means by "a correct escaping function"? What are the potential breakout strings you have to check for, and are they all caught by the standard Postgres API escaping functions in PHP, Tcl etc? For example, with SQL Server, it appears you can slip in a single-quote using encodings that are cast to ASCII #39 by the server. Also, what about pg functions like convert()? Could they be used to smuggle in a breakout character? Looked at like this, perhaps robust escaping is not so simple? Any advice much appreciated. ------------------ Geoff Caplan Vario Software Ltd (+44) 121-515 1154
In article <6.1.2.0.1.20040728170935.01f2ef38@localhost>,
Lincoln Yeoh <lyeoh@pop.jaring.my> writes:
> Just curious on what are the correct/official ways for dealing with
> the following scenarios:
> 1) Input string contains % and _ and would be used in a LIKE query
> that should not have user controllable wildcards.
Perhaps you mean something like the following:
my $sth = $dbh->prepare (q{
SELECT whatever
FROM mytable
WHERE somecol LIKE ? || '%'
});
$sth->execute ($input);
Even if $input contains '%' or '_', those characters get properly escaped.
> 2) Input string are numbers which can hold negative and positive
> values and could potentially be used in the following query:
> update tablea set a=10-$inputstring where key=1;
> When I tested at least one version of DBD::Pg doesn't appear to escape
> the inputstring when it's a number.
> e.g.
> $SQL="update tablea set a=10-? where key=1";
> And $SQL is used in a prepared statement with $inputstring as a parameter.
> I found that when $inputstring contained a negative number, all rows
> in tablea are set to the same value.
What exactly did you do? I tried
$sth = $dbh->prepare (q{
UPDATE t1
SET val = 10-?
WHERE id = ?
});
$sth->execute (-1, 1);
and it surely set val to 11.
> 3) Postgresql queries with select .... from ... where ... LIMIT $inputstring
The same thing:
$sth = $dbh->prepare ("SELECT id, val FROM t1 ORDER BY id LIMIT ?");
$sth->execute ($inputstring);
Harald Fuchs writes
> Perhaps you mean something like the following:
>
> my $sth = $dbh->prepare (q{
> SELECT whatever
> FROM mytable
> WHERE somecol LIKE ? || '%'
> });
> $sth->execute ($input);
>
> Even if $input contains '%' or '_', those characters get properly escaped.
Hum, what makes you think that? if $input is "_foo%", then the DBD
driver will produce this query:
SELECT whatever FROM mytable WHERE somecol like '_foo%'||'%'
The % and _ characters aren't escaped at all.
That can be confirmed by setting $dbh->trace_level to something greater or equal
than 2 and looking at the Pg DBD driver's output.
--
Daniel
PostgreSQL-powered mail user agent and storage: http://www.manitou-mail.org
At 05:30 PM 7/28/2004 +0200, Harald Fuchs wrote:
>Even if $input contains '%' or '_', those characters get properly escaped.
What versions are you using?
The versions I'm using are a bit old.
DBD::Pg 1.22 (3 versions old)
Postgresql 7.3.4
Schema+data:
create table test3 (a integer, b text);
insert into test3 (a,b) values (1,'a');
insert into test3 (a,b) values (2,'b');
insert into test3 (a,b) values (3,'c');
insert into test3 (a,b) values (4,'d');
code:
#!/usr/bin/perl -wT
use strict;
use DBI();
my ($DBNAME,$DBUSER,$DBPASS)=('DB','user','pass');
my $dbh = DBI->connect('DBI:Pg(AutoCommit =>
0):dbname='.$DBNAME,$DBUSER,$DBPASS
);
dbdo("update test3 set a=1-? where a=4","error testing",-1);
$dbh->commit;
$dbh->disconnect();
exit 0;
sub dbdo {
my $SQL=shift||'';
# dolog('DEBUG',$SQL);
my $errmsg=shift||'Unable to run database query!';
my $sth=$dbh->prepare($SQL) or die($errmsg.'|'.$DBI::errstr);
# dolog('DEBUG','prepared');
my $rv='';
$rv=$sth->execute(@_) or die($errmsg.'|'.$DBI::errstr);
# dolog('DEBUG','executed');
return ($sth,$rv);
}
In article <20040728184609.1900596@uruguay.brainstorm.fr>,
"Daniel Verite" <daniel@manitou-mail.org> writes:
> Harald Fuchs writes
>> Perhaps you mean something like the following:
>>
>> my $sth = $dbh->prepare (q{
>> SELECT whatever
>> FROM mytable
>> WHERE somecol LIKE ? || '%'
>> });
>> $sth->execute ($input);
>>
>> Even if $input contains '%' or '_', those characters get properly escaped.
> Hum, what makes you think that? if $input is "_foo%", then the DBD
> driver will produce this query:
> SELECT whatever FROM mytable WHERE somecol like '_foo%'||'%'
> The % and _ characters aren't escaped at all.
> That can be confirmed by setting $dbh->trace_level to something greater or equal
> than 2 and looking at the Pg DBD driver's output.
Shit, you're right. The $dbh->quote() called for the placeholders
escapes strings for INSERTing, but not for LIKE comparisons. So this
is one of the few places where using placeholders is not enough.
At least my erroneous assumption can't be used for an SQL injection
attack - you just get more results than you would get if you escape
the wildcards by hand.
> > That can be confirmed by setting $dbh->trace_level to something greater > or equal > > than 2 and looking at the Pg DBD driver's output. > >Shit, you're right. The $dbh->quote() called for the placeholders >escapes strings for INSERTing, but not for LIKE comparisons. So this >is one of the few places where using placeholders is not enough. > >At least my erroneous assumption can't be used for an SQL injection >attack - you just get more results than you would get if you escape >the wildcards by hand. I've been reading this discussion and I asked myself whether you guys remove/replace unwanted chars from strings you get from the web or not.. If you do remove them AFAIK it doesn't only prevent SQL injection but also XSS. B.
In article <6.0.0.22.0.20040729123957.02ac5b70@pop.atz.nl>, "B. van Ouwerkerk" <bvo@atz.nl> writes: > I've been reading this discussion and I asked myself whether you guys > remove/replace unwanted chars from strings you get from the web or > not.. The problem is not limited to strings you get from the web. Those strings can come from _any_ source you don't control fully. And you don't remove unwanted chars - a search for "O'Neill" is prefectly reasonable and not more dangerous than a search for "Anderson" as long as you escape the quotation mark properly. > If you do remove them AFAIK it doesn't only prevent SQL injection but also XSS. You can prevent XSS in the same manner: carefully escape everything that looks dangerous. You just use different escaping rules because you have other dangerous characters (especially '<').