RecursionError in pyclbr.readmodule_ex

[baltsers]

Bug report

We did a fuzzing test on Python3.9.15, a recursion error happened.

from pyclbr import *
import pyclbr
import io
import tokenize
from token import DEDENT
from token import NAME
from token import OP

class demoCls():

    def __init__(self):
        pass

    def demoFunc(self, arg1,arg2):
        try:
            ret = pyclbr.readmodule_ex(arg1,arg2)
        except (AttributeError, ImportError, ModuleNotFoundError, OSError, StopIteration, TypeError, ValueError, tokenize.StopTokenizing, tokenize.TokenError):
            pass


module = '.'* 100_100
path = None
dc = demoCls()
dc.demoFunc(module, path)

details

Bug info

Traceback (most recent call last):
  File "/root/fuzzVuls/Python3.9.15/pyclbr.readmodule_ex/1#pyclbr#readmodule_ex.py", line 24, in <module>
    dc.demoFunc(module, path)
  File "/root/fuzzVuls/Python3.9.15/pyclbr.readmodule_ex/1#pyclbr#readmodule_ex.py", line 16, in demoFunc
    ret = pyclbr.readmodule_ex(arg1,arg2)
  File "/root/anaconda3/lib/python3.9/pyclbr.py", line 116, in readmodule_ex
    return _readmodule(module, path or [])
  File "/root/anaconda3/lib/python3.9/pyclbr.py", line 149, in _readmodule
    parent = _readmodule(package, path, inpackage)
  File "/root/anaconda3/lib/python3.9/pyclbr.py", line 149, in _readmodule
    parent = _readmodule(package, path, inpackage)
  File "/root/anaconda3/lib/python3.9/pyclbr.py", line 149, in _readmodule
    parent = _readmodule(package, path, inpackage)
  [Previous line repeated 993 more times]
  File "/root/anaconda3/lib/python3.9/pyclbr.py", line 140, in _readmodule
    if module in sys.builtin_module_names and inpackage is None:
RecursionError: maximum recursion depth exceeded in comparison

My environment

CPython: 3.9.15
Ubuntu: 18.0.4 (x86 64)

[gaogaotiantian]

Another meaningless expected behavior...

[gaogaotiantian]

Yes pyclbr._readmodule() read submodules recursively and if you have 100000 dots it will exceed maximum recursion depth.

[zware]

@baltsers, can we discuss what you're doing here? You have yet to respond to any comments on issues you have opened, and it's becoming problematic. Please respond in this issue before you open any others.

[baltsers]

The behavior is expected at the interpreter side, but it is unexpected for the application. The API documentation does not provide any indication of input limitations. This could be exploited by attackers to cause the application to crash. Same for this issue#103800.

[gaogaotiantian]

Are you suggesting that we should document every single Exception a function could potentially raise? For example, math.sqrt could raise a TypeError if you send in a string, and ValueError if the argument is negative. In both cases, the attacker could exploit the "vulnerability" to crash the application? I don't think it's the interpreter's duty to make sure all the applications could survive attacks from user inputs. Also I don't think an issue that can be solved with a try ... except ... block is a vulnerability. Exceptions exists for a reason.

[terryjreedy]

Our bug report template requests "A clear and concise description of what the bug is." This and the previous RecursionError report do not state what you consider the bug to be. That a function given an impossible to handle input gracefully exits with an appropriate exception and truthful message? That we use recursion?

Your message suggested that the 'bug' is inadequate documentation. All builtin-in exceptions are documented in https://docs.python.org/3/library/exceptions.html. I could be persuaded that the RecursionError entry could be expanded to state 1. recursion errors usually arise from processing sufficiently long repetitive structure with recursion and 2. that they can be avoided (sometimes) by a) using iteration instead of recursion, b) increasing the recursion limit, and c) testing input size. Methods b) and c) work together. It only took me a few minutes to determine the smalled n for _readmodule fails with IDLE on Windows: n=1019; r('.a'*n, []). No realistic dotted name will exceed this.

We intentionally do not try to list all possible exceptions for every function. RecursionError, in particular, is implementation-specific and not a Python behavior. This is true of input limitations in general.

We do not consider an application exiting with a report of an uncaught error to be a 'crash'. A crash is python exit without an exception, with the system generating a core dump or crash box. Unlike some crashes, exiting after printing a traceback and exception message is not a security issue.

For testing Python, please use the current main branch or at least the most recent release of the most recent version -- currently 3.11. 3.10 and older only get security fixes and there have been many bugfixes since the last 3.10 bugfix release and even more since the last 3.9 bugfix release.

[baltsers]

Sorry for the confusion.

1. Our concern is not about the documentation issue; but the robustness and reliability of the runtime API implementation, which can significantly impact the applications that use them.
2. From the perspective of application developers, they do not know enough about such hidden implementation limitations, which can easily cause unhandled exceptions in the applications, further exploited by attackers. The solution "try ..... except...." can help avoid this problem from happening. However, in application development, we usually want to handle different exceptions in specified solutions, so this blanket treatment is not ideal for developers.
3. These days, while testing the runtime APIs, we actually found plenty of non-functional errors, for example, Memory errors, Integer Overflow, and Recursion errors, thrown by the interpreter; since these errors have not been handled by the APIs, then being leaked to the application level, how can an application developer write robust code without the knowledge of such low-level details?

[gaogaotiantian]

In my opinion, there are two separate categories of issues:

1. A function fails when a reasonable input is given - it's a bug, it will affect all the developers using that function
2. A function that causes a security issue when any form of input is given - it's a vulnerability that can be exploited by attackers

Your issues definitely do not fall into category 1, and it would be a very weak argument to say they belong to category 2.

Basically, your issues are saying - if I feed this function an arbitrary input from the users, it'll raise an exception (or crash on allocating a huge amount of memory which Python can't guard against). What's your ideal behavior? If you think this is a bug, then we should have a better solution on that right? Feasible or not.

• We could never generate exceptions on any user input? Return an error code let user check against that?
• Or we could force user to check against all exceptions that could be possibly raised in the underlying code on the language level?
• We could document all the exceptions that could be raised by a function?
• We could limit the exceptions to a certain range? Only allow standard libraries to raise certain types of exceptions?

What's your expected behavior on this specific matter? Do you think ModuleNotFoundError or ImportError is more appropriate(even though they are not listed in the documentation either)? I don't think "there's an exception when I do this" is a bug report - it should be "I don't think there should be an exception" or "I think it should be another exception".

[sobolevn]

You can also solve this "issue" by increasing the recursion limit (if you really want to).

I agree that this seems to work as expected.
Garbage in - nice error out.