[Bf-committers] Patch: removing use of exec() and eval() from python source

[Mathias Panzenböck]

On 10/20/2009 10:29 PM, Campbell Barton wrote:
> Blender has a C function to get rna paths which I intended to use if
> exec/eval are a problem.
> another reason I did not write this is that it should support index
> lookups, not just attributes since "blah[1].foo[13]" is a valid rna
> sting, from what I gather your function does not support index lookups
> and assignments.
> 

Assignments are supported, though index lookups aren't. Are there only integer
indices or also associative arrays using strings or even floats as index?

> as for security, if you can change someones configuration to inject
> python code - you would already be able to modify their config in ways
> that do other bad things.
> for instance - keybindings are saved as a python script, so if you
> share keybindings, injecting code into an operator is no worse then
> what you could do in the python script to begin with.
> 

Yes, the possibility for injections isn't that of an issue. The resulting
exception when something goes wrong and the readability are bigger issues here
in my opinion.

Even if the code is not changed to use the rna lookup function I think it should
still be changed this way:
exec("context.%s='%s'" % (self.path, self.value))
->
exec("context.%s=%r" % (self.path, self.value))

Oh and I think this might introduce a precision loss (or doesn't it?):
exec("context.%s=%f" % (self.path, self.value))

And this is just plain... well obscure to a reader:
exec("context.%s = ['%s', '%s'][context.%s!='%s']" % (self.path, self.value_1,
self.value_2, self.path, self.value_2))

It should at least be changed to something like this to be more clear:
value = eval('context.%s' % self.path)
if value == self.value_2:
	exec('context.%s=%r' % (self.path, self.value_1))
else:
	exec('context.%s=%r' % (self.path, self.value_2))

Or in case python >= 2.5 can be assumed:
exec("context.%s = %r if context.%s!=%r else %r" % (self.path, self.value_2,
self.path, self.value_2, self.value_1))

Using '%s' might not just allow injections, it also might screw up unicode (or
non 7-bit ascii) characters. %r should encode these characters properly (so it's
independent of the codec the python parser uses).

	-panzi