From bfde49756524bd3748234dc6dfa8015e37176e9b Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 5 Nov 2013 14:23:58 +0100 Subject: [PATCH] rework login and discovery code to avoid strlen beyond end of data Checking for the presence of the NUL character should be done without accessing beyond the PDU datain. Use memchr instead of strlen, and compute the length only if a NUL character is actually there. Signed-off-by: Paolo Bonzini --- lib/discovery.c | 31 ++++++++++++++++++++----------- lib/login.c | 34 ++++++++++++++++++++++------------ 2 files changed, 42 insertions(+), 23 deletions(-) diff --git a/lib/discovery.c b/lib/discovery.c index 52b7762..e12a15a 100644 --- a/lib/discovery.c +++ b/lib/discovery.c @@ -118,25 +118,34 @@ iscsi_process_text_reply(struct iscsi_context *iscsi, struct iscsi_pdu *pdu, pdu->private_data); return -1; } + if (size == 0) { + iscsi_set_error(iscsi, "size == 0 when parsing " + "discovery data"); + pdu->callback(iscsi, SCSI_STATUS_ERROR, NULL, + pdu->private_data); + return -1; + } - while (size > 0) { + do { + unsigned char *end; int len; - len = strlen((char *)ptr); - - if (len == 0) { - break; - } - - if (len > size) { - iscsi_set_error(iscsi, "len > size when parsing " - "discovery data %d>%d", len, size); + end = memchr(ptr, 0, size); + if (end == NULL) { + iscsi_set_error(iscsi, "NUL not found after offset %ld " + "when parsing discovery data", + ptr - in->data); pdu->callback(iscsi, SCSI_STATUS_ERROR, NULL, pdu->private_data); iscsi_free_discovery_addresses(iscsi, targets); return -1; } + len = end - ptr; + if (len == 0) { + break; + } + /* parse the strings */ if (!strncmp((char *)ptr, "TargetName=", 11)) { struct iscsi_discovery_address *target; @@ -187,7 +196,7 @@ iscsi_process_text_reply(struct iscsi_context *iscsi, struct iscsi_pdu *pdu, ptr += len + 1; size -= len + 1; - } + } while (size > 0); pdu->callback(iscsi, SCSI_STATUS_GOOD, targets, pdu->private_data); iscsi_free_discovery_addresses(iscsi, targets); diff --git a/lib/login.c b/lib/login.c index aebb084..9a7347f 100644 --- a/lib/login.c +++ b/lib/login.c @@ -981,30 +981,40 @@ iscsi_process_login_reply(struct iscsi_context *iscsi, struct iscsi_pdu *pdu, if (iscsi_serial32_compare(expcmdsn, iscsi->expcmdsn) > 0) { iscsi->expcmdsn = expcmdsn; } - + + if (size == 0) { + iscsi_set_error(iscsi, "size == 0 when parsing " + "login data"); + pdu->callback(iscsi, SCSI_STATUS_ERROR, NULL, + pdu->private_data); + return -1; + } + /* XXX here we should parse the data returned in case the target * renegotiated some some parameters. * we should also do proper handshaking if the target is not yet * prepared to transition to the next stage */ - while (size > 0) { + do { + char *end; int len; - len = strlen(ptr); - - if (len == 0) { - break; - } - - if (len > size) { - iscsi_set_error(iscsi, "len > size when parsing " - "login data %d>%d", len, size); + end = memchr(ptr, 0, size); + if (end == NULL) { + iscsi_set_error(iscsi, "NUL not found after offset %ld " + "when parsing login data", + (unsigned char *)ptr - in->data); pdu->callback(iscsi, SCSI_STATUS_ERROR, NULL, pdu->private_data); return -1; } + len = end - ptr; + if (len == 0) { + break; + } + /* parse the strings */ if (!strncmp(ptr, "TargetAddress=", 14)) { strncpy(iscsi->target_address,ptr+14,MAX_STRING_SIZE); @@ -1077,7 +1087,7 @@ iscsi_process_login_reply(struct iscsi_context *iscsi, struct iscsi_pdu *pdu, ptr += len + 1; size -= len + 1; - } + } while (size > 0); if (status == SCSI_STATUS_REDIRECT && iscsi->target_address[0]) { ISCSI_LOG(iscsi, 2, "target requests redirect to %s",iscsi->target_address);