Skip to content

[FIXED] Check of maxpayload could be bypassed if size overruns int32#1053

Merged
kozlovic merged 1 commit intomasterfrom
mpay_overrun
Jul 1, 2019
Merged

[FIXED] Check of maxpayload could be bypassed if size overruns int32#1053
kozlovic merged 1 commit intomasterfrom
mpay_overrun

Conversation

@kozlovic
Copy link
Copy Markdown
Member

@kozlovic kozlovic commented Jul 1, 2019

One could craft a PUB protocol to cause server to panic. This can
happen if the size in the PUB protocol overruns an int32.

(note that if authorization is enabled, the user would need to
authenticate first, limiting the impact).

Thank you to Aviv Sasson and Ariel Zelivansky from Twistlock
for the security report!

Signed-off-by: Ivan Kozlovic ivan@synadia.com

@kozlovic kozlovic requested a review from derekcollison July 1, 2019 20:45
Copy link
Copy Markdown
Member

@derekcollison derekcollison left a comment

Choose a reason for hiding this comment

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

Checking on int32 -> int64 cast removing negative bit.

Also should credit TL for report.

Comment thread server/client.go
Copy link
Copy Markdown
Member

@derekcollison derekcollison left a comment

Choose a reason for hiding this comment

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

LGTM

One could craft a PUB protocol to cause server to panic. This can
happen if the size in the PUB protocol overruns an int32.

(note that if authorization is enabled, the user would need to
authenticate first, limiting the impact).

Thank you to Aviv Sasson and Ariel Zelivansky from Twistlock
for the security report!

Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
@kozlovic
Copy link
Copy Markdown
Member Author

kozlovic commented Jul 1, 2019

Added to commit log reference to people from Twistlock that reported the security issue.

@kozlovic kozlovic merged commit a171864 into master Jul 1, 2019
@kozlovic kozlovic deleted the mpay_overrun branch July 1, 2019 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants