7
\$\begingroup\$

I wrote a python module that needs to load a file parser to work. Initially it was just one text parsing module but I need to add more parsers for different cases.

parser_class1.py

parser_class2.py

parser_class3.py

Only one is required for every running instance. I'm thinking load it by command line:

mmain.py -p parser_class1

I wrote this code in order to select the parser to load when the main module is called:

#!/usr/bin/env python

import argparse
aparser = argparse.ArgumentParser()
aparser.add_argument('-p',
            action='store',
            dest='module',
            help='-i module to import')
results = aparser.parse_args()

if not results.module:
    aparser.error('Error! no module')
try:
    exec("import %s" %(results.module))
    print '%s imported done!'%(results.module)
except ImportError, e:
    print e

But, I was reading that this way is dangerous, maybe not standard.

Then is this approach ok?

Or must I find another way to do it?

Why?

\$\endgroup\$
4
\$\begingroup\$

Regarding the safety aspect of your question.

The reason why exec() can be dangerous is that is can allow a nefarious agent to execute code that you never intended.

Let's assume for example that somewhere in your program, you have sensitive data elements such as:

username = secret_username
password = never_share

And let's also assume that someone calls your program like this:

mmain.py -p 'parser_class1;print globals()'

Then your exec() statement would actually be:

exec("import %s" %('parser_class1;print globals()'))

And this would result in your program printing out all variables in your global space for them... including your username and password.

By making your program utilize the __import__ method as mentioned by @Jaime, you can at least prevent people from executing non-import statements in your code.

But, you should, whenever possible also examine the input from a user before using it to execute any dynamic code.

\$\endgroup\$
3
\$\begingroup\$

Sanitising your inputs with argparse

As has already been mentioned in comments and @eikonomega’s answer, running arbitrary code from the user is generally dangerous. Using __import__ or importlib restricts their ability somewhat.

You can make it even safer by restricting the set of available modules, by passing a choices flag to add_argument. Like so:

aparser.add_argument('-p',
            action='store',
            dest='module',
            help='-i module to import',
            choices=['parser1', 'parser2', 'parser3'])

argparse will now enforce that only the values "parser1", "parser2" or "parser3" can be entered here – anything else will cause it to exit and print a usage message, e.g.:

$ python conditional_imports.py -p foo
usage: conditional_imports.py [-h] [-p {parser1,parser2,parser3}]
conditional_imports.py: error: argument -p: invalid choice: 'foo' (choose from 'parser1', 'parser2', 'parser3')

It’s also better for the end-user, because the help/usage message will tell them the available options, rather than leaving them to guess.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.