Fix handling of malicious PDV size #87
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It was possible to overflow length in "length -= 4 + pdvLength;" when pdvLength is set to a value greater then LONG_MAX + pduLength. By doing that the length overflowed and became positive, even if it was initially less than 4 + pdvLength.
By doing this we avoid warning '<': signed/unsigned mismatch. This can also prevent problems when pduLength is greater than long.
|
Hi, since we are in the process of preparing a release right now, we don't want to integrate any serious changes in the networking code. We will integrate the patch after the release, i.e. probably in 2-3 weeks. |
|
Since this is security-related, we decided to integrate this into the upcoming release. See commit 1549d8c. Thanks for your report and patch! |
|
@michaelonken, that's great news, thank you! Could you please also have a look at #79 when you have time after release? |
|
Sure, w already have an internal feature branch that expanded on the work of #79. No pull request gets lost ;) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It was possible to overflow the length variable in DT_2_IndicatePData on line "length -= 4 + pdvLength;" when pdvLength is set by a server to a value greater then LONG_MAX + pduLength. By doing that the length overflowed and became positive, even if it was initially less than 4 + pdvLength. Due to the overflow "if (pdvLength < 2)" was not working and in the next loop iteration EXTRACT_LONG_BIG was causing read access violation.
Added check "length < 4 + pdvLength" to solve this problem.
Added check ULONG_MAX - pdvLength < 4 to make sure pdvLength + 4 will not overflow unsigned long.