Skip to content

guard AstroidError.__str__ against user text in the message - #3214

Closed
kali834x wants to merge 1 commit into
pylint-dev:mainfrom
kali834x:error-str-format-injection
Closed

guard AstroidError.__str__ against user text in the message#3214
kali834x wants to merge 1 commit into
pylint-dev:mainfrom
kali834x:error-str-format-injection

Conversation

@kali834x

@kali834x kali834x commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Type of Changes

Type
🐛 Bug fix

Description

AstroidError.str runs self.message.format(**vars(self)), but many astroid errors interpolate untrusted source text into the message first, so an invalid identifier like namedtuple("{0}", "abc") ends up re-read as a str.format template. {0} raises IndexError, {message.foo} raises AttributeError, and a width field such as {message:200000000} builds a huge string, all during inference and none of them caught by the existing except ValueError, so the exception escapes node.infer() and crashes the caller. keeping the self-referential message out of the substitution fields and widening the fallback stops injected braces from crashing or allocating, while the {field!r} message templates still render.

Closes #3199

@codspeed-hq

codspeed-hq Bot commented Aug 9, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 3 untouched benchmarks
⏩ 1 skipped benchmark1


Comparing kali834x:error-str-format-injection (d481582) with main (00c1ba2)

Open in CodSpeed

Footnotes

  1. 1 benchmark was skipped, so the baseline result was used instead. If it was deleted from the codebase, click here and archive it to remove it from the performance reports.

@codecov

codecov Bot commented Aug 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.65%. Comparing base (00c1ba2) to head (d481582).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #3214   +/-   ##
=======================================
  Coverage   93.64%   93.65%           
=======================================
  Files          93       93           
  Lines       11574    11575    +1     
=======================================
+ Hits        10839    10840    +1     
  Misses        735      735           
Flag Coverage Δ
linux 93.51% <100.00%> (+<0.01%) ⬆️
pypy 93.65% <100.00%> (+<0.01%) ⬆️
windows 93.62% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
astroid/exceptions.py 95.86% <100.00%> (+0.03%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Pierre-Sassoulas

Copy link
Copy Markdown
Member

Thank you for this PR. #3203 by @Sanjays2402 predates this and fixes a subset of the same bug I told them to credit you for the performance improvment and I'm going to close this as duplicate..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

IndexError: Replacement index 0 out of range for positional args tuple

2 participants