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
msg/msg_types: fix the entity_addr_t's decoder #17699
Conversation
src/msg/msg_types.h
Outdated
@@ -456,7 +456,8 @@ struct entity_addr_t { | |||
__u32 elen; | |||
::decode(elen, bl); | |||
if (elen) { | |||
bl.copy(elen, (char*)get_sockaddr()); | |||
bl.copy(std::min((size_t)elen, get_sockaddr_len()), | |||
(char*)get_sockaddr()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would it make more sense to throw buffer::error if elen > get_sockaddr_len()? That seems better than partially decoding the (bogus) input.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@liewegas makes sense, i will check all the caller sites , and add the error handling code if necessary.
introduced by 6a7fe5a |
854aba5
to
c23a326
Compare
@liewegas fixed and repushed. |
c23a326
to
01ba79f
Compare
the daemon is vulnerable to malicious client, which is able to send large elen, and corrupt the stack, etc. * throw at seeing corrupted entity_addr_t where its elen exceeds the length of sockaddr * handle the exception thrown when decoding entity_addr_t in messenger layer. * if a malicious client manages to send a corrutped entity_addr_t to daemon, daemon will crash because decode fails and the exception is not handled. it's better than continuing working with the bogus message. Signed-off-by: Kefu Chai <kchai@redhat.com>
01ba79f
to
aa83c2c
Compare
the daemon is vulnerable to malicious client, which is able to send
large elen, and corrupt the stack, etc.
Signed-off-by: Kefu Chai kchai@redhat.com