Skip to content

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