I'm using exec and I hear that's bad...

This is the place for queries that don't fit in any of the other categories.

I'm using exec and I hear that's bad...

Postby TheFunk » Tue Sep 24, 2013 7:01 pm

I'm writing a simple application to run a few SQL Server Metrics and log the outputs to a text file. I've been trying to avoid writing sloppy or bad code, but it's not been going too well. In this case, I need to use exec to execute code that has been stored as a string.

The string in question:
Code: Select all
 def conndb(self, server, database, usernm, passwd):
        global connstring     # I hate global variables
        connstring = 'cnxn = pyodbc.connect(r\''+'DRIVER={SQL Server Native Client 10.0};SERVER='+server+';DATABASE='+database+';Trusted_Connection=yes;User ID='+usernm+';PWD='+passwd+'\')'
        print(connstring)     # For debugging purposes, I check the connection string


Which is then referenced by:
Code: Select all
 def runmetrics(self):
        exec(connstring)
        cursor = cnxn.cursor()
        data = open('info.txt', 'w')


There must be a better way. I know I can knock off a few lines of this code somehow, but at the moment, the biggest problem is that exec. I hear using exec is a major no-no. How do I kill it?
TheFunk
 
Posts: 27
Joined: Fri Aug 30, 2013 5:46 pm

Re: I'm using exec and I hear that's bad...

Postby ochichinyezaboombwa » Tue Sep 24, 2013 8:00 pm

What is the point of having your connstr? Why not just
Code: Select all
cnxn = pyodbc.connect(...)


In other words:
Code: Select all
def conndb(self, server, database, usernm, passwd):
    self.cnxn = pyodbc.connect(...)

and then
Code: Select all
def runmetrics(self):
    cursor = self.cnxn.cursor()
    ....
ochichinyezaboombwa
 
Posts: 200
Joined: Tue Jun 04, 2013 7:53 pm

Re: I'm using exec and I hear that's bad...

Postby micseydel » Tue Sep 24, 2013 9:12 pm

exec is considered "bad" because it executes arbitrary Python code. That means if that Python code would wipe your computer, you're SOL. So it's "bad" if you take strings from potentially malicious users on the internet and execute them. In fact that's more than bad, that's terrible.

If the strings are Python code you've written, and you're sure that they're safe, then as long as wherever you store them is as secure as your script, then you're as safe as you can be.

If the Python strings you are executing are relatively regular or expectable, you can simply parse them to get the information you want and then execute your own known Python code instead of arbitrary Python. That would also make it easier to detect attempts at abuse, as well as obviously making it more difficult to abuse your system. It's definitely better habit to write the more robust and secure code, of course.
Join the #python-forum IRC channel on irc.freenode.net!

Please do not PM members regarding questions which are meant to be discussed publicly. The point of the forum is so that others can benefit from it. We don't want to help you over PMs or emails.
User avatar
micseydel
 
Posts: 1390
Joined: Tue Feb 12, 2013 2:18 am
Location: Mountain View, CA

Re: I'm using exec and I hear that's bad...

Postby TheFunk » Wed Sep 25, 2013 1:33 pm

Being a network security major, I completely get what you're saying micseydel. Actually the connstring variable was originally going to be so that I could sanitize input and prevent SQL Injection in the connection string, but I realize now that's not necessary, and using exec could result in some interesting commands getting run on the machine running my script, which is just as bad!

pyodbc has it's own means of preventing SQL injection (http://code.google.com/p/pyodbc/wiki/GettingStarted see parameters) and ochichinyezaboombwa's suggestion works well with how my program is set up.

So now exec is gone and my code reads:

Code: Select all
   def conndb(self, server, database, usernm, passwd):
        self.cnxn = pyodbc.connect(r'DRIVER={SQL Server Native Client 10.0};SERVER=?;DATABASE=?;Trusted_Connection=yes;User ID=?;PWD=?', var1=server, var2=database, var3=usernm, var4=passwd)



I had to use keyword arguments because for some reason pyodbc.connect wasn't taking positional arguments(I think that has something to do with lambda and my GUI), and this code still throws an error when connecting (hardcoding the variables makes it work perfectly though, so at least I know it can connect). Anyway, that's an entirely different issue, which I'll either eventually figure out, or ask about in another thread. Thanks for the help! I'm getting better with this, slowly but surely.

No evals or execs going on around here. I'd rather not end up with an rm -rf or del system32 situation on my hands.
TheFunk
 
Posts: 27
Joined: Fri Aug 30, 2013 5:46 pm


Return to General Coding Help

Who is online

Users browsing this forum: Google [Bot], Guitarman2010 and 4 guests

cron