eval is used, which is unsafe

[eric-wieser]

In the following places:

• calibration/calibration_estimation/src/calibration_estimation/single_transform.py

  Line 48 in 20d3bbf

  eval_config = [eval(str(x)) for x in config]

• calibration/calibration_estimation/src/calibration_estimation/joint_chain.py

  Line 52 in 20d3bbf

  self._axis = numpy.matrix([ eval(str(x)) for x in config['axis']], float)

• calibration/calibration_estimation/src/calibration_estimation/urdf_params.py

  Line 144 in 20d3bbf

  transform = [eval(str(x)) for x in transform]

What type of input is this trying to parse?

[eric-wieser]

All the cases seem to be eval(str(x)). Is this:

• Trying to be copy.copy?
• Trying to be float?
• Deliberately trying to support expression input in text form(ie, 1+2*3?

Whichever this it, it's a bad idea for two reasons:

• Pushing floats through string form is inefficient and precision losing
• You probably don't want to execute arbitrary python code passed in from a config file

[vrabaud]

Well, I don't know that Python style. That's a question to ask @mikeferguson :) Thx

[mikeferguson]

This actually predates me -- I just moved things around and tried to parameterize and clean up -- in particular I think this is for lines like: https://github.com/PR2/pr2_calibration/blob/hydro-devel/pr2_calibration_launch/estimate_params/config_pr2_beta/system.yaml#L109 where it has to evaluate "pi/2"

[vrabaud]

good catch ! That .yaml is a custom format right ? Maybe we can just parse pi properly and that''s it.
eval can be a security issue like ros-perception/vision_opencv#112

[eric-wieser]

Is pi the only global that should be accessible in the expression? Or should we expose all of math.*? Right now, there are way more constants accesible than makes sense. Something like this would work:

https://stackoverflow.com/questions/2371436/evaluating-a-mathematical-expression-in-a-string