What is meta? ×
Meta Stack Exchange is where users like you discuss bugs, features, and support issues that affect the software powering all 128 Stack Exchange communities.

One topic that seems to pop up quite frequently is that the parameter implementation in SEDE allows for SQL injection.

To address this I added typed parameters yesterday, which means you can let us know, when you define the query if a param is an int, a float or a string.

Here is an example of typed params

Out of the box I still allow for the trivial string replace params like: ##whatever##

The thing is that this upsets certain people in particular people feel uncomfortable with stuff like this

The way I see it the whole text box where anonymous can enter whatever query they want is injection central anyway. The security is implemented by running everything given in by users under a restricted account. Now if we had a system where query runners were not allowed to author queries this becomes a totally valid concern.

The reason I added the typed params was to make it easier for people to have text params and not have to escape things like O'malley

My question is:

Are we doing enough? Should we be doing more in the "sql injection protection" department?

share|improve this question
    
Is there a date/time format? Also I note you can't leave a string blank, but one space will do for most SQL cases. –  Mark Hurd Jun 26 '10 at 6:01
    
float doesn't recognise exponential notation i.e. 1e-7 fails and yet 0.0000001 is displayed as the former. –  Mark Hurd Jun 26 '10 at 16:23
add comment

2 Answers 2

up vote 4 down vote accepted

I support typed params, but I think extending to SQL Injection Protection thing is overdoing, as the anonymous can enter whatever query they want in the composition text area anyway. (Or I don't understand what do you mean by "a system where query runners were not allowed to author queries".)

share|improve this answer
1  
Sorry, I mean that since it is open source somebody may fork it and use it in a different environment where not everybody has permission to author queries, totally agree with your prognosis –  waffles Jun 25 '10 at 11:32
add comment

I don't think there's a sane way to prevent SQL injection except to properly set up the permissions on the database itself (which should be done regardless in any application).

You've already taken care of that, so I don't think there's anything more to worry about.

In terms of someone else using the code, the readme could contain an explicit warning to make sure that the database permissions get set up, or they risk destructive queries going through. I should point out that although it might be a rare occurrence, allowing destructive queries may actually be desired in someone's implementation. I think that's probably the best we can do -- give fair and ample warning. If that advice is not heeded, well, tough luck really.

Edit: With regards to disallowing authoring queries, the simplest route is probably to only allow authoring to registered users (or a subset thereof defined by the site owner). This is probably best implemented by using a set of restricted accounts, one for each permission level, with suitable UI changes for basic deterrence.


Obligatory xkcd:

alt text

share|improve this answer
add comment

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .