[zeromq-dev] [PATCH] Corrected discarding of remainder of message when request ID invalid
Perry Kundert
perry at kundert.ca
Tue Oct 11 21:52:39 CEST 2011
>From 84c3c5b97944aedcd6c50a384cb9243d273f57d2 Mon Sep 17 00:00:00 2001
From: Perry Kundert <perry at kundert.ca>
Date: Tue, 11 Oct 2011 13:33:06 -0600
Subject: [PATCH] Corrected discarding of remainder of message when request
ID invalid
When zmq::req_t::xrecv detects that a response has no request ID
label, or the ID is the wrong size, it would return an EAGAIN, but
would not discard the remainder of the message. This could allow the
remainder of the message to incorrectly "leak" into a future response,
if it is crafted to look like a reply with a valid response ID.
Discard all remaining message blocks, if the ID is invalid in any way.
Signed-off-by: Perry Kundert <perry at kundert.ca>
---
src/req.cpp | 21 ++++-------
tests/test_invalid_rep.cpp | 82
+++++++++++++++++++++++++++++++++++++++++--
2 files changed, 86 insertions(+), 17 deletions(-)
diff --git a/src/req.cpp b/src/req.cpp
index 04a19fb..c4ec546 100644
--- a/src/req.cpp
+++ b/src/req.cpp
@@ -78,7 +78,7 @@ int zmq::req_t::xsend (msg_t *msg_, int flags_)
int zmq::req_t::xrecv (msg_t *msg_, int flags_)
{
- // If request wasn't send, we can't wait for reply.
+ // If request wasn't sent, we can't wait for reply.
if (!receiving_reply) {
errno = EFSM;
return -1;
@@ -91,20 +91,15 @@ int zmq::req_t::xrecv (msg_t *msg_, int flags_)
return rc;
// TODO: This should also close the connection with the peer!
- if (unlikely (!(msg_->flags () & msg_t::label) || msg_->size () !=
4)) {
- errno = EAGAIN;
- return -1;
- }
-
- unsigned char *data = (unsigned char*) msg_->data ();
- if (unlikely (get_uint32 (data) != request_id)) {
-
- // The request ID does not match. Drop the entire message.
- while (true) {
+ // If invalid, ensure we drop remainder and return an empty message
+ if (unlikely (!(msg_->flags () & msg_t::label))
+ || unlikely (msg_->size () != 4)
+ || unlikely (get_uint32 ((unsigned char *)msg_->data())
+ != request_id)) {
+ // The request ID is bad or doesn't match. Drop the entire
message.
+ while (msg_->flags () & (msg_t::label | msg_t::more)) {
int rc = xreq_t::xrecv (msg_, flags_);
errno_assert (rc == 0);
- if (!(msg_->flags () & (msg_t::label | msg_t::more)))
- break;
}
msg_->close ();
msg_->init ();
diff --git a/tests/test_invalid_rep.cpp b/tests/test_invalid_rep.cpp
index 2657c20..90f69b5 100644
--- a/tests/test_invalid_rep.cpp
+++ b/tests/test_invalid_rep.cpp
@@ -20,6 +20,7 @@
#include "../include/zmq.h"
#include <assert.h>
+//#include <stdio.h>
int main (int argc, char *argv [])
{
@@ -55,11 +56,79 @@ int main (int argc, char *argv [])
rc = zmq_recv (xrep_socket, body, sizeof (body), 0);
assert (rc == 1);
- // Send invalid reply.
+ // Send invalid reply (addr not label).
rc = zmq_send (xrep_socket, addr, 4, 0);
assert (rc == 4);
- // Send valid reply.
+ rc = zmq_recv (req_socket, body, sizeof (body), ZMQ_DONTWAIT);
+ assert (rc == -1);
+
+ // Send invalid reply (ID not label).
+ rc = zmq_send (xrep_socket, addr, 4, ZMQ_SNDLABEL);
+ assert (rc == 4);
+ rc = zmq_send (xrep_socket, seqn, 4, 0); // right ID, not
label!
+ assert (rc == 4);
+
+ rc = zmq_recv (req_socket, body, sizeof (body), ZMQ_DONTWAIT);
+ assert (rc == -1);
+
+ // Send invalid reply (bad ID labels, followed by good label, invalid
data),
+ // ensuring that we always discard entire message (and not leak), if ID
+ // is found to be invalid.
+ rc = zmq_send (xrep_socket, addr, 4, ZMQ_SNDLABEL); // right address
+ assert (rc == 4);
+ rc = zmq_send (xrep_socket, seqn, 3, ZMQ_SNDLABEL); // wrong ID size
+ assert (rc == 3);
+ rc = zmq_send (xrep_socket, seqn, 3, ZMQ_SNDLABEL); // ''
+ assert (rc == 3);
+ rc = zmq_send (xrep_socket, seqn, 3, ZMQ_SNDLABEL); // ''
+ assert (rc == 3);
+ rc = zmq_send (xrep_socket, seqn, 3, ZMQ_SNDLABEL); // ''
+ assert (rc == 3);
+ rc = zmq_send (xrep_socket, seqn, 3, ZMQ_SNDLABEL); // ''
+ assert (rc == 3);
+ rc = zmq_send (xrep_socket, seqn, 3, ZMQ_SNDLABEL); // ''
+ assert (rc == 3);
+ rc = zmq_send (xrep_socket, seqn, 4, ZMQ_SNDLABEL); // right ID!
+ assert (rc == 4);
+ rc = zmq_send (xrep_socket, "v", 1, 0); // but bad data
+ assert (rc == 1);
+
+ rc = zmq_recv (req_socket, body, sizeof (body), ZMQ_DONTWAIT);
+ assert (rc == -1);
+
+ // Send invalid reply (bad ID label, followed by good label, invalid
data).
+ rc = zmq_send (xrep_socket, addr, 4, ZMQ_SNDLABEL); // right address
+ assert (rc == 4);
+ rc = zmq_send (xrep_socket, addr, 4, ZMQ_SNDLABEL); // wrong ID value
+ assert (rc == 4);
+ rc = zmq_send (xrep_socket, seqn, 4, ZMQ_SNDLABEL); // right ID!
+ assert (rc == 4);
+ rc = zmq_send (xrep_socket, seqn, 4, ZMQ_SNDLABEL); // ''
+ assert (rc == 4);
+ rc = zmq_send (xrep_socket, "w", 1, 0); // but bad data
+ assert (rc == 1);
+
+ rc = zmq_recv (req_socket, body, sizeof (body), ZMQ_DONTWAIT);
+ assert (rc == -1);
+
+ // Send invalid reply (not label, followed by good label, invalid
data).
+ rc = zmq_send (xrep_socket, addr, 4, ZMQ_SNDLABEL); // right address
+ assert (rc == 4);
+ rc = zmq_send (xrep_socket, seqn, 4, ZMQ_SNDMORE); // right ID, but
not a label
+ assert (rc == 4);
+ rc = zmq_send (xrep_socket, addr, 4, ZMQ_SNDLABEL); // right address
+ assert (rc == 4);
+ rc = zmq_send (xrep_socket, seqn, 4, ZMQ_SNDLABEL); // right ID!
+ assert (rc == 4);
+ rc = zmq_send (xrep_socket, "x", 1, 0); // but bad data
+ assert (rc == 1);
+
+ rc = zmq_recv (req_socket, body, sizeof (body), ZMQ_DONTWAIT);
+ assert (rc == -1);
+
+
+ // Send valid reply.
rc = zmq_send (xrep_socket, addr, 4, ZMQ_SNDLABEL);
assert (rc == 4);
rc = zmq_send (xrep_socket, seqn, 4, ZMQ_SNDLABEL);
@@ -67,10 +136,15 @@ int main (int argc, char *argv [])
rc = zmq_send (xrep_socket, "b", 1, 0);
assert (rc == 1);
- // Check whether we've got the valid reply.
+ // Check whether we've got only the (final) valid reply (and no more).
rc = zmq_recv (req_socket, body, sizeof (body), 0);
assert (rc == 1);
- assert (body [0] == 'b');
+ //printf( "body: %c\n", *body );
+ assert (body [0] == 'b');
+
+ rc = zmq_recv (req_socket, body, sizeof (body), ZMQ_DONTWAIT);
+ assert (rc == -1);
+
// Tear down the wiring.
rc = zmq_close (xrep_socket);
--
1.7.2.2
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.zeromq.org/pipermail/zeromq-dev/attachments/20111011/51de08c2/attachment.htm>
More information about the zeromq-dev
mailing list