Is This Open To SQL Injection?

[Stephen Hansen]

On 7/7/10 11:52 AM, Stephen Hansen wrote:
> On 7/7/10 11:38 AM, Victor Subervi wrote:
>> Hi;
>> I have this code:
>>
>>     sql = 'insert into personalDataKeys values (%s, %s, %s)' % (store,
>> user, ', %s'.join('%s' * len(col_vals))
>>     cursor.execute(sql, col_vals)
> 
> First, its always best to be explicit with insert statements. Meaning,
> don't rely on the underlining structure of a table, as in:
> 
> INSERT INTO YourRandomTable VALUES ("my", "value", "here");
> 
> Instead, do:
> 
> INSERT INTO YourRandomTable (field1, field2, field3) VALUES ("my",
> "value", "here");

I suddenly feel a need to come back and explain *why* I make the claim
to 'best' above: the explicit naming of the fields in the INSERT,
specifically, since others have shown how to do the INSERT safely while
keeping the essentially variable number of items in the values clause.

I still would advise against that approach even if it is safe from a SQL
Injection standpoint: but for a different reason entirely, that of
long-term maintainability.

No design is perfect; no customer specification (no matter how vetted,
analyzed, and approved by stakeholders) survives implementation and
real-life usage.

If you always select specific columns in a specific order (i.e., always
SELECT this, that, other; and never SELECT *), and always insert with
your columns specified (i.e., always INSERT INTO blah (this, that,
other) and never INSERT INTO blah VALUES (..)), then it lets you adapt
your application in the future when something comes up.

Specifically, it lets you add new columns without breaking everything :)
Now, those new columns would need to either allow NULL's or have a
default value of course.

But some day down the road you can go and do an ALTER TABLE to add say,
"my_whatever" to the above, and you don't suddenly have to vet every
single piece of code which accesses that table. All the existing code
will still work: its getting the pieces of data it knows how to use. As
you need to, you can adjust that code to take into account this new
piece of data. But by making any new additions "optional" in your SQL,
and making all your other accesses explicit, it just eases migration and
maintenance in future updates.

Some may disagree, I dunno. I just find in my experience that following
that practice has saved a lot of time and effort down the road.
(Especially during migrations from old versions to new versions, and
running versions concurrently during some test-phase, etc, or rolling
back a new version if a critical bug is found: the changes made to the
database to support the new versions can safely persist without you
having to do a much more expensive / time-consuming restoration of the
database from a backup).

-- 

   Stephen Hansen
   ... Also: Ixokai
   ... Mail: me+list/python (AT) ixokai (DOT) io
   ... Blog: http://meh.ixokai.io/

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 487 bytes
Desc: OpenPGP digital signature
URL: <http://mail.python.org/pipermail/python-list/attachments/20100708/4672d607/attachment-0001.sig>