Skip to content

Crash when using multiple capturing groups in re.Scanner #140797

Closed

Description

@T90REAL

Bug report

Bug description:

The re.Scanner class crashes with a segmentation fault (memory corruption) when any pattern in the lexicon contains named capturing groups.

import re

lexicon = [
    ('(?P<group1>a)(?P<group2>b)', None),  # Named capturing groups
]

scanner = re.Scanner(lexicon)
result, leftover = scanner.scan("ab")  # Segmentation fault occurs here
zsh: segmentation fault  python3 test.py

Scanner should process the input normally and return scan results without crashing, but Python interpreter crashes with segmentation fault, indicating memory corruption.

CPython versions tested on:

3.12

Operating systems tested on:

Linux

Linked PRs

Activity

  1. wjssz commented on Oct 31, 2025

    @wjssz

    1️⃣ p.state.groups is 2:

    cpython/Lib/re/__init__.py

    Lines 405 to 406 in ac1ffd7

    p = _parser.SubPattern(s, [(BRANCH, (None, p))])
    self.scanner = _compiler.compile(p)

    2️⃣ p.state.groups-1 is 1:

    cpython/Lib/re/_compiler.py

    Lines 778 to 782 in ac1ffd7

    return _sre.compile(
    pattern, flags | p.state.flags, code,
    p.state.groups-1,
    groupindex, tuple(indexgroup)
    )

    3️⃣ PatternObject.groups is 1:

    self->groups = groups;

    4️⃣ Then malloc state->mark with size 2:

    state->mark = PyMem_New(const void *, pattern->groups * 2);

    5️⃣ Here state->mark[i] = ptr, i is 3, which is out of bounds:

    state->mark[i] = ptr;

    6️⃣ PyMem_Free() found state->mark has been written out of bounds.

    PyMem_Free((void*) state->mark);

    In step 1️⃣, p.state.groups value (2) is incorrect, this led to the crash.

    re.Scanner is an undocumented class introduced in July 2000: 7cafe4d
    re.Scanner's usage is unclear, maybe it can be removed?

  2. picnixz commented on Oct 31, 2025

    @picnixz
    Member

    It is undocumented but not unuseful (it is marked as experimental). I think we should add an assert when setting mark[i] and store the original array size if possible. I don't have access to my laptop now so this will need to wait.

    Btw, was it an LLM-generated answer?

  3. wjssz commented on Oct 31, 2025

    @wjssz

    It's unuseful, the words "experimental stuff (see python-dev discussions for details)" exist since 2000.

    Btw, was it an LLM-generated answer?

    It was written by me, not by AI.

  4. devdanzin commented on Oct 31, 2025

    @devdanzin
    Member

    See #106338 (and links) for a proposal about documenting it.

  5. serhiy-storchaka commented on Oct 31, 2025

    @serhiy-storchaka
    Member

    re.Scanner is a pure Python class, but it uses private API to create compiled pattern from bytecode. There is a validation of bytecode, but it is not enough in this case. Anyway, re.Scanner does not work if subpatterns contain capturing groups. So, we can do two things:

    • Make validation more strict.
    • Forbid any capturing groups in re.Scanner.

    We can also discuss deprecation of re.Scanner, but this is a different issue. It is still here in such state because we don;t know what to do with it.

  6. wjssz commented on Oct 31, 2025

    @wjssz

    Forbid capturing group is simple. No need news entry and unit-tests.

    Such functionality is suitable for being placed in a PyPI module, which would result in better functionality and quality.

  7. picnixz commented on Oct 31, 2025

    @picnixz
    Member

    Forbid capturing group is simple. No need news entry and unit-tests.

    I disagree here. It does not cost us that much to add tests and NEWS, though forbidding the capturing group is indeed easy since we have the pattern object. But any change of functionality that could break existing code should be announced, even for undocumented APIs as there might be code relying on it. The class is not even privately named.

    Offloading to a third party lib is a separate issue that must be discussed separately as well.

  8. serhiy-storchaka commented on Oct 31, 2025

    @serhiy-storchaka
    Member

    Other issues related to re.Scanner() and capturing groups: #56998 and #84440. See also #49587 (comment). Capturing groups never worked with re.Scanner().

  9. wjssz commented on Nov 1, 2025

    @wjssz

    @T90REAL, would you be willing to submit a PR? Forbid capturing groups.

  10. Abhi210 commented on Nov 1, 2025

    @Abhi210
    Contributor

    If @T90REAL does not mind, I could work on this issue.

  11. T90REAL commented on Nov 1, 2025

    @T90REAL
    Author

    @T90REAL, would you be willing to submit a PR? Forbid capturing groups.
    If @T90REAL does not mind, I could work on this issue.

    Sure, you could work on that. Probably add a check in the initialization step of the re.Scanner ?

  12. picnixz commented on Nov 1, 2025

    @picnixz
    Member

    I would prefer Serhiy working on it as he assigned the issue to himself. In general when an issue is already assigned to someone, it is left at their discretion whethwr to delegate or not

  13. serhiy-storchaka commented on Nov 2, 2025

    @serhiy-storchaka
    Member

    I tried to implement more strict validation but this would cost too much. Anyway, if you only use the documented public API, the code is always valid. And when you create code in nonstandard way, you are responsible for its validity.

    On the other hand, banning capture groups in Scaner is a trivial task. Feel free to take it.

  14. removed their assignment
    on Nov 2, 2025
  15. Abhi210 commented on Nov 3, 2025

    @Abhi210
    Contributor

    @serhiy-storchaka @picnixz I have added the following tests for forbidding the capturing groups:

    def test_bug_140797(self):
            #bug 140797: remove capturing groups compilation form re.Scanner
    
            #Presence of Capturing group throws an error
            lex = [("(a)b", None)]
            with self.assertRaises(ValueError):
                Scanner(lex)
    
            #Presence of non-capturing groups should pass normally
            s = Scanner([("(?:a)b", lambda scanner, token: token)])
            result, rem = s.scan("ab")
            self.assertEqual(result,['ab'])
            self.assertEqual(rem,'')
    
            #Testing a very complex capturing group
            pattern= "(?P<name>[A-Za-z]+)*="
            with self.assertRaises(ValueError):
                Scanner([(pattern, None)])
    
            #Testing the pattern given by the user
            lexicon = [('(?P<group1>a)(?P<group2>b)', None)]
            with self.assertRaises(ValueError):
                Scanner(lexicon)
    

    Are there any other tests which you would like to add here?

  16. serhiy-storchaka commented on Nov 3, 2025

    @serhiy-storchaka
    Member

    It should be: if (arg >= 2 * groups) {

    Agree. Do you mind to create a PR?

  17. Abhi210 commented on Nov 3, 2025

    @Abhi210
    Contributor

    Sorry, I haven't signed the CLA agreement. Welcome others to take over that issue.

    Can you create a new issue and add an easy tag for others to work on this? Otherwise, I can create a new PR here, but this is a different issue.

  18. added 7 commits that reference this issue on Nov 3, 2025
  19. added a commit that references this issue on Nov 4, 2025
  20. added 2 commits that reference this issue on Nov 4, 2025
  21. added 3 commits that reference this issue on Nov 4, 2025
  22. StanFromIreland commented on Dec 3, 2025

    @StanFromIreland
    Member

    This change has broken people's scripts, see the comments in duplicate #142220.

  23. wjssz commented on Dec 3, 2025

    @wjssz

    re.Scanner is an undocumented class, changing its behavior in 3.13.10/3.14.1 should be ok.

    Its implementation doesn't support capturing group. The captured string is unreliable.

    Use non-capturing parentheses to fix the problem:

    - (r'.*?((?<!\\),)', lambda x,t: t[:-1]),
    + (r'.*?(?:(?<!\\),)', lambda x,t: t[:-1]),
  24. serhiy-storchaka commented on Dec 3, 2025

    @serhiy-storchaka
    Member

    This can work by accident, if the number of the captured groups is less than the sequential number of the phrase (counting from 1). No capturing group is allowed in the first phrase, one capturing group can work in the second phrase, two in the third phrase, etc. This is because when the scanner handles group N for phrase N, previous N-1 groups are unused, and setting them will not conflict with group N and will not affect lastgroup.

    For example, the code reported in #142220 worked for that reason, because the second phrase contains only one capturing group.

        arg_parser = re.Scanner([
                    (r',', lambda x,t: ''),
                    (r'.*?((?<!\\),)', lambda x,t: t[:-1]),
                    (r'.*?\)', lambda x,t: t[:-1]),
            ])

    I am not sure that this always works if such condition is satisfied, and in any case such condition is too artificial.

    But there is a code in the wild that "works", and we should not break it in the 3.13 bugfix.

    We can simply revert the Scanner change in 3.13. We can also raise an exception conditionally, or emit a warning, or make re.Scanner supporting arbitrary capturing groups, but I am not sure it is worth a hassle.

    It is perhaps fine for 3.14, taking into account that re.Scanner is undocumented.

  25. added 2 commits that reference this issue on Dec 3, 2025
  26. eli-schwartz commented on Dec 5, 2025

    @eli-schwartz
    Contributor

    @StanFromIreland Seems a bit weird to me to mix two completely different topics in one ticket and close a perfectly natural report as a stigmatic "duplicate"...

    It is perhaps fine for 3.14, taking into account that re.Scanner is undocumented.

    @serhiy-storchaka given the code worked fine in 3.14 and was tested in several release candidates and then released into the wild in 3.14.0 final, I must say that I don't see any rationale for backporting to 3.14 any more than 3.13 (or rather let us say, any rationale to refrain from reverting the backport).

    It's the same issue in both. People have already started using 3.14, and packaging and deploying scripts that work on 3.14.0, and this backport broke that. Given that, again, my argument in the ticket closed as a duplicate (??) was that instead of backporting a raise Exception('this is not allowed') for code that could work, it would be better to only backport a deprecation warning or something, I think this well applies to 3.14 too.

    The "solution" for multiple capturing groups causing crashes, was simply to ban doing things that are useless anyway. No code was actually made to work -- the only difference was a clearer and more understandable error message (an exception instead of a crash) so nobody's code would actually work without fixing the code anyway.

    That being the case, it is not really an urgent priority to change the type of error in released versions of CPython if it also changes working scripts into erroring scripts... and that rule applies to 3.14.1 as well.

    And again, adding the exception on main but adding a deprecation warning on released stable versions seems quite reasonable. A deprecation warning can still be discovered in CI and responded to, but doesn't break scripts in the wild.

  27. added a commit that references this issue on Dec 5, 2025
  28. serhiy-storchaka commented on Dec 5, 2025

    @serhiy-storchaka
    Member

    @hugovk, what do you say?

  29. hugovk commented on Dec 5, 2025

    @hugovk
    Member

    Let's also revert for 3.14.2, then we have a couple of months to decide on whether to reintroduce the fix, or deprecate first, or something else.

  30. added a commit that references this issue on Dec 5, 2025
  31. hugovk commented on Dec 5, 2025

    @hugovk
    Member

    I've opened #142309 to revert for 3.14.

  32. added a commit that references this issue on Dec 5, 2025
  33. serhiy-storchaka commented on Dec 5, 2025

    @serhiy-storchaka
    Member

    I am closing this issue. #140984 fixed the original crash. #140944 improved error detection and perhaps saved from other errors, but since it broke some user code, the backports were reverted.

  34. added a commit that references this issue on Dec 6, 2025
  35. added a commit that references this issue on Dec 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    extension-modulesC modules in the Modules dirtopic-regextype-crashA hard crash of the interpreter, possibly with a core dump

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions