Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

closes bpo-42938: Replace snprintf with Python unicode formatting in ctypes param reprs. #24239

Merged
merged 1 commit into from
Jan 18, 2021

Conversation

benjaminp
Copy link
Contributor

@benjaminp benjaminp commented Jan 18, 2021

@benjaminp benjaminp merged commit 916610e into python:master Jan 18, 2021
@miss-islington
Copy link
Contributor

Thanks @benjaminp for the PR 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7, 3.8, 3.9.
🐍🍒⛏🤖

@benjaminp benjaminp deleted the buffer branch January 18, 2021 20:47
@miss-islington
Copy link
Contributor

Sorry, @benjaminp, I could not cleanly backport this to 3.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 916610ef90a0d0761f08747f7b0905541f0977c7 3.7

@miss-islington
Copy link
Contributor

Sorry @benjaminp, I had trouble checking out the 3.6 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 916610ef90a0d0761f08747f7b0905541f0977c7 3.6

hroncok pushed a commit to fedora-python/cpython that referenced this pull request Feb 1, 2021
00357 #
CVE-2021-3177: Replace snprintf with Python unicode formatting in ctypes param reprs

Backport of Python3 commit 916610e:
https://bugs.python.org/issue42938
python#24239
@ericonr
Copy link

ericonr commented Feb 18, 2021

Isn't the commit message here rather misleading? These weren't snprintf calls, which is exactly what allowed them to overflow. If this PR did what it claims to do, this would only be a bug fix for truncated precision when printing some values, not a security patch.

One can clearly see in the code that sprintf is being used, not snprintf.

@jennes
Copy link

jennes commented Feb 19, 2021

Isn't the commit message here rather misleading? These weren't snprintf calls, which is exactly what allowed them to overflow. If this PR did what it claims to do, this would only be a bug fix for truncated precision when printing some values, not a security patch.

One can clearly see in the code that sprintf is being used, not snprintf.

It is. But I guess this is to be interpreted as "even better than just fixing it with the second best solution, which would be snprintf"

@ahesford
Copy link

It is. But I guess this is to be interpreted as "even better than just fixing it with the second best solution, which would be snprintf"

That would be the wrong interpretation. To replace means to take the place of, and snprintf has no place in the affected changes.

algitbot pushed a commit to alpinelinux/aports that referenced this pull request Feb 19, 2021
bpo-42938: Replace snprintf with Python unicode formatting in ctypes param reprs.

This is a backport of python/cpython#24239 sourced
from https://salsa.debian.org/saifelse/python2/-/commit/cac0240f5b8d8460f5e3217fd23de256109b0847
hroncok pushed a commit to fedora-python/cpython that referenced this pull request Apr 19, 2021
00357 #
CVE-2021-3177: Replace snprintf with Python unicode formatting in ctypes param reprs

Backport of Python3 commit 916610e:
https://bugs.python.org/issue42938
python#24239
hroncok pushed a commit to fedora-python/cpython that referenced this pull request May 19, 2021
00357 #
CVE-2021-3177: Replace snprintf with Python unicode formatting in ctypes param reprs

Backport of Python3 commit 916610e:
https://bugs.python.org/issue42938
python#24239
stratakis pushed a commit to stratakis/cpython that referenced this pull request Jul 18, 2022
00357 #
CVE-2021-3177: Replace snprintf with Python unicode formatting in ctypes param reprs

Backport of Python3 commit 916610e:
https://bugs.python.org/issue42938
python#24239
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs backport to 3.9 only security fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants