Skip to content

bpo-35050: AF_ALG length check off-by-one error #10058

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

Merged
merged 3 commits into from
Dec 10, 2018

Conversation

tiran
Copy link
Member

@tiran tiran commented Oct 23, 2018

The length check for AF_ALG salg_name and salg_type had a off-by-one
error. The code assumed that both values are not necessarily NULL
terminated. However the Kernel code for alg_bind() ensures that the last
byte of both strings are NULL terminated.

Signed-off-by: Christian Heimes christian@python.org

https://bugs.python.org/issue35050

The length check for AF_ALG salg_name and salg_type had a off-by-one
error. The code assumed that both values are not necessarily NULL
terminated. However the Kernel code for alg_bind() ensures that the last
byte of both strings are NULL terminated.

Signed-off-by: Christian Heimes <christian@python.org>
@tiran tiran force-pushed the bpo-35050-af_alg_null branch from 2a5e820 to 149840e Compare October 23, 2018 13:22
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM, but

@@ -0,0 +1 @@
Fix off-by-one bug in length check for AF_ALG name and type.
Copy link
Member

Choose a reason for hiding this comment

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

please mention the socket module somewhere

@bedevere-bot
Copy link

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

* reformat a comment
* close the socket in the unit test
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

@tiran: I modified your PR to fix the 3 minor issues that I spotted.

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM.

@vstinner vstinner merged commit 2eb6ad8 into python:master Dec 10, 2018
@miss-islington
Copy link
Contributor

Thanks @tiran for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry, @tiran and @vstinner, I could not cleanly backport this to 3.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 2eb6ad8578fa9d764c21a92acd8e054e3202ad19 3.7

@miss-islington
Copy link
Contributor

Sorry, @tiran and @vstinner, I could not cleanly backport this to 3.6 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 2eb6ad8578fa9d764c21a92acd8e054e3202ad19 3.6

@bedevere-bot
Copy link

GH-11069 is a backport of this pull request to the 3.7 branch.

@bedevere-bot
Copy link

GH-11070 is a backport of this pull request to the 3.6 branch.

vstinner added a commit that referenced this pull request Dec 10, 2018
The length check for AF_ALG salg_name and salg_type had a off-by-one
error. The code assumed that both values are not necessarily NULL
terminated. However the Kernel code for alg_bind() ensures that the last
byte of both strings are NULL terminated.

Signed-off-by: Christian Heimes <christian@python.org>
(cherry picked from commit 2eb6ad8)
vstinner added a commit that referenced this pull request Dec 10, 2018
The length check for AF_ALG salg_name and salg_type had a off-by-one
error. The code assumed that both values are not necessarily NULL
terminated. However the Kernel code for alg_bind() ensures that the last
byte of both strings are NULL terminated.

Signed-off-by: Christian Heimes <christian@python.org>
(cherry picked from commit 2eb6ad8)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants