[Python-Dev] Improved evaluator added to ast module

[Benjamin Peterson]

2012/10/11 Vinay Sajip <vinay_sajip at yahoo.co.uk>:
> Benjamin Peterson <benjamin <at> python.org> writes:
>
>>
>> With this operations, you can still cause a lot of trouble.
>>
>
> Perhaps; I am hoping that some more specific information (about the kind of
> trouble this can cause) will emerge. Hence the request for review.

For example, if you have attribute access you can easily get to
important modules like "sys" and "os". Banning function calls is
essentially a red-herring, since almost any operation in Python can
cause arbitrary code execution.

>
>> What exactly are you trying to prevent?
>
> The issue was raised because people felt that the use of unrestricted eval()
> in logging.config.fileConfig() - usage intended to convert class names and
> constructor arguments to Python objects suitable for actually creating objects
> like logging handlers - was a potential security hole, because configurations
> can be sent and received over the network (not from remote clients, but there's
> still a potential vulnerability for machines with multiple users like non-VPS
> hosting boxes). No specific hole was described, so I don't have a specific list
> of things I'm trying to prevent, other than function calls.

Surely, if a hacker can get the app to use logging configs he sends,
he can have it send sensitive log data to himself.

>
> I added support for the items I thought would be useful in an evaluator slightly
> more capable than literal_eval, but it's quite possible that I've allowed more
> things than needed (e.g. array indexing and slicing). Clearly, replacing eval()
> with literal_eval() will mean potential breakage of code in config files out
> there - but that's a trade-off that may need to be made if security
> considerations prevail.
>
> Alternatively, it may be considered that the changes I've already committed
> to logging's listen() function - to allow a verifier callback to be specified
> - is sufficient to allay the concerns that led to the issue being raised in the
> first place. In which case, I can close the issue without committing this patch.

I like that. I don't exactly unterstand the usecase for logging
untrusted logging configurations.


-- 
Regards,
Benjamin