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?

200_success
145k22 gold badges190 silver badges478 bronze badges
asked Mar 25, 2013 at 16:02
\$\endgroup\$
0

2 Answers 2

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.

answered Oct 22, 2013 at 17:34
\$\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.

answered Feb 19, 2017 at 10:27
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.