[issue23967] Make inspect.signature expression evaluation more powerful

[Brett Cannon]

Change by Brett Cannon :


--
nosy:  -brett.cannon

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue23967] Make inspect.signature expression evaluation more powerful

[Eric Wieser]

Eric Wieser  added the comment:

> To make this work I had to write an ast printer that produces evaluatable 
> Python code.  Note that it's not complete, I know it's not complete, it's 
> missing loads of operators.  Assume that if this is a good idea I will add 
> all the missing operators.

Now that `ast.unparse` is in (bpo-38870), can this patch be simplified?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue23967] Make inspect.signature expression evaluation more powerful

[Eric Wieser]

Change by Eric Wieser :


--
nosy: +Eric Wieser

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue23967] Make inspect.signature expression evaluation more powerful

[Nick Coghlan]

Nick Coghlan added the comment:

Right, Larry and I had a fairly long discussion about this idea at the sprints, 
and I was satisfied that all the cases where he's proposing to use this are 
safe: in order to exploit them you need to be able to set __text_signature__ on 
arbitrary objects, and if an attacker can do that, you've already lost control 
of the process.

However, a natural future extension is to expose this as a public alternative 
constructor for Signature objects, and for that, the fact that it ultimately 
calls eval() under the hood presents more of a security risk. The 
trusted=False default on _signature_fromstr allows the function to be used 
safely on untrusted data, while allowing additional flexibility when you *do* 
trust the data you're evaluating.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23967
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue23967] Make inspect.signature expression evaluation more powerful

[Larry Hastings]

Larry Hastings added the comment:

It's only used for signatures in builtins.  Any possible security hole here is 
uninteresting because the evil hacker already got to run arbitrary C code in 
the module init.

Because it's only used for signatures in builtins, we shouldn't encounter a 
function with a mutable default value like {} or [] which gets mutated later.  
Builtins don't have those.

In case you're wondering about the trusted parameter, that was suggested by 
Nick Coghlan at the PyCon sprints.  He's thinking that other callers may use 
_signature_fromstr() in the future, and he wanted the API to make it clear that 
future uses may be on non-trustworthy sources.

And, finally, consider that the original version already calls eval(). 
Admittedly it uses eval() in a way that should be much harder to exploit.  But 
it's not an enormous difference between the two calls.

I don't really think we need to post to python-dev about this.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23967
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue23967] Make inspect.signature expression evaluation more powerful

[Larry Hastings]

Larry Hastings added the comment:

Cleaned up the patch some more--the code was stupid in a couple places.  I 
think it's ready to go in.

--
Added file: 
http://bugs.python.org/file39181/larry.improved.signature.expressions.3.txt

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23967
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue23967] Make inspect.signature expression evaluation more powerful

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Using complex expressions is deceitful. In Python functions the default value 
is evaluated only once, at function creation time, but inspect.signature will 
evaluate it every time. For example foo(x={}) and foo(x=dict()) means the same 
in function declaration, but different in signature.

It could also affect security, because allow arbitrary code execution at the 
place where it was not allowed before.

I think this issue should be discussed on Python-Dev. I'm not sure that it is 
pythonic.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23967
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue23967] Make inspect.signature expression evaluation more powerful

[Larry Hastings]

Larry Hastings added the comment:

Whoops.  Here's the revised patch.

--
Added file: 
http://bugs.python.org/file39123/larry.improved.signature.expressions.2.txt

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23967
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue23967] Make inspect.signature expression evaluation more powerful

[Larry Hastings]

Larry Hastings added the comment:

Thanks to #24002 I now know how to write evalify_node properly.  This patch is 
now much better.

Note that I deliberately made the new function _eval_ast_expr() as a private 
module-level routine.  I need that same functionality in Argument Clinic too, 
so if both patches are accepted I'll have Clinic switch to calling this version.

--
nosy: +brett.cannon

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23967
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue23967] Make inspect.signature expression evaluation more powerful

[Larry Hastings]

Larry Hastings added the comment:

I should mention that evalify_node() is pretty hacked up here, and is not ready 
to be checked in.  (I'm proposing separately that we simply add something like 
this directly into the standard library, see issue #24002.)

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23967
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue23967] Make inspect.signature expression evaluation more powerful

[Peter McCormick]

Peter McCormick added the comment:

This definitely works for the _socket.listen use case!

In terms of generating such a signature using Argument Clinic, currently this 
is required:

backlog: int(py_default=builtins.min(SOMAXCONN, 128), 
c_default=Py_MIN(SOMAXCONN, 128)) = 000


The attached patch lets Tools/clinic/clinic.py make an exception when both C 
and Python defaults are specified, simplifying the above to: 

backlog: int(py_default=builtins.min(SOMAXCONN, 128), 
c_default=Py_MIN(SOMAXCONN, 128))

--
keywords: +patch
Added file: 
http://bugs.python.org/file39066/pdm-argument_clinic-mixed_py_and_c_defaults-v1.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23967
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue23967] Make inspect.signature expression evaluation more powerful

[Peter McCormick]

Peter McCormick added the comment:

I missed the fact that Larry's patch obviates the need for the `builtins.` 
prefix, shortening the Argument Clinic parameter specification into:

backlog: int(py_default=min(SOMAXCONN, 128), c_default=Py_MIN(SOMAXCONN, 
128))

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23967
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue23967] Make inspect.signature expression evaluation more powerful

[Larry Hastings]

New submission from Larry Hastings:

Peter's working on converting socket to use Argument Clinic.  He had a default 
that really should look like this:

min(SOME_SOCKET_MODULE_CONSTANT, 128)

min wasn't something we'd needed before.  I thought about it and realized we 
could do a much better job of simulating the evaluation context of a shared 
module.

Initially I thought, all I needed was to bolster the environment we used for 
eval() to add the builtins.  (Which I've done.)  But this wasn't sufficient 
because we deliberately used ast.literal_eval(), which doesn't support function 
calls by design for superior security.  Or subscripting, or attribute access.  
We already worked around those I think.

But how concerned are we about security?  What is the attack vector here?  If 
the user is able to construct an object that has a villainous 
__text_signature__ on it... surely they could already do as they like?

So here's a first draft at modifying the __text_signature__ evaluation 
environment so it can handle much more sophisticated expressions.  It can use 
anything from builtins, or anything in sys.modules, or anything in the current 
module; it can call functions, and subscript, and access attributes, and 
everything.

To make this work I had to write an ast printer that produces evaluatable 
Python code.  Note that it's not complete, I know it's not complete, it's 
missing loads of operators.  Assume that if this is a good idea I will add all 
the missing operators.

Nick was worried that *in the future* we might expose a turn this string into 
a signature function.  That might make an easier attack vector.  So he asked 
that the trusted= keyword flag be added, and the full-on eval only happen if 
the string is trusted.

--
assignee: larry
components: Library (Lib)
files: larry.improved.signature.expressions.1.txt
messages: 241140
nosy: larry, ncoghlan, pdmccormick, serhiy.storchaka, yselivanov, zach.ware
priority: normal
severity: normal
stage: patch review
status: open
title: Make inspect.signature expression evaluation more powerful
type: enhancement
versions: Python 3.5
Added file: 
http://bugs.python.org/file39047/larry.improved.signature.expressions.1.txt

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23967
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com