Skip to content

[3.6] bpo-31852: Fix segfault caused by using the async soft keyword#4122

Merged
vstinner merged 4 commits intopython:3.6from
pablogsal:bpo31852
Oct 31, 2017
Merged

[3.6] bpo-31852: Fix segfault caused by using the async soft keyword#4122
vstinner merged 4 commits intopython:3.6from
pablogsal:bpo31852

Conversation

@pablogsal
Copy link
Copy Markdown
Member

@pablogsal pablogsal commented Oct 25, 2017 •
edited
Loading

This PR solves a segmentation fault in Python 3.6 caused by a combination of the async soft keyword and continuation lines. Steps to reproduce:

>>> async \
...
  File "<stdin>", line 1
    \ufffd\ufffdF\ufffd\ufffd
         ^
SyntaxError: invalid syntax
>>> async \
Segmentation fault

As @Haypo mentioned in the issue you can use this file to use the issue in the tokenizer to induce a buffer overflow. This PR solves this issue as well.

The current implementation checks if the current token is ASYNC and sets a sentient value (2) in the tok->async_def before looking for the token ahead (which is the step where the segfault happens). The value of tok->async_def gets overwritten after the lookahead by the usual value (1). As this particular issues are fixed by #1669 in the current master (3.7) this PR acts as a mere patch.

https://bugs.python.org/issue31852

vstinner
vstinner reviewed Oct 26, 2017
View reviewed changes
Comment thread Parser/tokenizer.c Outdated
/* Line continuation */
if (c == '\\') {
c = tok_nextc(tok);
if ( tok->async_def == 2){
Copy link
Copy Markdown
Member

@vstinner vstinner Oct 26, 2017

Choose a reason for hiding this comment

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

Coding style, please write: "if (...) {" (fix spacing)

Copy link
Copy Markdown
Member Author

@pablogsal pablogsal Oct 26, 2017

Choose a reason for hiding this comment

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

Done in 7518077

@pablogsal
Copy link
Copy Markdown
Member Author

pablogsal commented Oct 26, 2017

Should I add a NEWS entry?

@vstinner
Copy link
Copy Markdown
Member

vstinner commented Oct 26, 2017 via email

@pablogsal
Copy link
Copy Markdown
Member Author

pablogsal commented Oct 27, 2017

Added in e5b1993

@pablogsal
Copy link
Copy Markdown
Member Author

pablogsal commented Oct 30, 2017

@Haypo There is anything more I should change?

@vstinner vstinner requested a review from 1st1 October 30, 2017 16:30
@vstinner
Copy link
Copy Markdown
Member

vstinner commented Oct 30, 2017

@1st1: Would you mind to review this PR please?

1st1
1st1 approved these changes Oct 30, 2017
View reviewed changes
Copy link
Copy Markdown
Member

@1st1 1st1 left a comment

Choose a reason for hiding this comment

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

I think this is an OK fix. Let's merge it.

@vstinner vstinner merged commit 690c36f into python:3.6 Oct 31, 2017
@vstinner
Copy link
Copy Markdown
Member

vstinner commented Oct 31, 2017

Thank you @pablogsal! I merged your PR.

@pablogsal pablogsal deleted the bpo31852 branch October 31, 2017 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner left review comments

@1st1 1st1 1st1 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@pablogsal @vstinner @1st1 @the-knights-who-say-ni @bedevere-bot