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.

Sign up for GitHub

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

Jump to bottom

bpo-36495: Fix two out-of-bounds array reads #12641

Merged
merged 3 commits into from
Apr 1, 2019
Merged

bpo-36495: Fix two out-of-bounds array reads #12641

merged 3 commits into from
Apr 1, 2019

Conversation

bradlarsen
Copy link
Contributor

@bradlarsen bradlarsen commented Mar 31, 2019 •
edited by bedevere-bot
Loading

@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

Thanks again for your contribution, we look forward to reviewing it!

serhiy-storchaka
serhiy-storchaka reviewed Mar 31, 2019
View reviewed changes
Misc/NEWS.d/next/Security/2019-03-31-15-06-35.bpo-36495.ahXWSI.rst Outdated
@@ -0,0 +1 @@
Fix two out-of-bound reads in the code that constructs abstract syntax trees. Patch by Brad Larsen.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure this fix needs a NEWS entry, especially in the Security section. The bug was introduces at the alpha stage, nobody should use it in production.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree; I was originally going to leave that blank, but bedevere-bot said one was needed (or I didn't understand how to skip that check).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I revert the commit that adds the NEWS entry?

@bradlarsen
Copy link
Contributor Author

It looks like that Azure Pipelines Ubuntu job has been usually failing for a while now?

@tirkarthi
Copy link
Member

It looks like that Azure Pipelines Ubuntu job has been usually failing for a while now?

It's a known issue tracked at #12625

gvanrossum
gvanrossum approved these changes Apr 1, 2019
View reviewed changes
Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution! Great research. Let's fix these, but let's get rid of the NEWS item.

@bradlarsen
Revert "📜🤖 Added by blurb_it."
3a69b87 
This reverts commit 6f90ef3.

No need for a NEWS entry on a prerelease bugfix.
@bradlarsen
Copy link
Contributor Author

Great! I've reverted the NEWS item commit, so I think this PR is ready now.

@gvanrossum gvanrossum merged commit a4d7836 into python:master Apr 1, 2019
@bradlarsen bradlarsen deleted the fix-issue-36495 branch April 7, 2019 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka left review comments

@gvanrossum gvanrossum gvanrossum approved these changes

Assignees
No one assigned
Labels
skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@bradlarsen @the-knights-who-say-ni @tirkarthi @gvanrossum @serhiy-storchaka @bedevere-bot