Re: [libvirt] [PATCH 4/5] phyp: Fix too small buffer allocation in phypAttachDevice

sizeof(domain->name) is the wrong thing. Instead of using strdup here rewrite escape_specialcharacters to allocate the buffer itself. Add a contains_specialcharacters to be used in phypOpen, as phypOpen is not interested in the escaped version. --- src/phyp/phyp_driver.c | 90 ++++++++++++++++++++++++++++++++--------------- 1 files changed, 61 insertions(+), 29 deletions(-) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 24165fb..226946d 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -891,14 +891,63 @@ phypUUIDTable_Free(uuid_tablePtr uuid_table) VIR_FREE(uuid_table); } -static int -escape_specialcharacters(char *src, char *dst, size_t dstlen) +static bool +contains_specialcharacters(const char *src) { size_t len = strlen(src); - char temp_buffer[len]; - unsigned int i = 0, j = 0; + size_t i = 0; + if (len == 0) - return -1; + return false; + + for (i = 0; i < len; i++) { + switch (src[i]) { + case '&': + case ';': + case '`': + case '@': + case '"': + case '|': + case '*': + case '?': + case '~': + case '<': + case '>': + case '^': + case '(': + case ')': + case '[': + case ']': + case '{': + case '}': + case '$': + case '%': + case '#': + case '\\': + case '\n': + case '\r': + case '\t': + return true; + } + } + + return false; +} + +static char * +escape_specialcharacters(const char *src) +{ + size_t len = strlen(src); + size_t i = 0, j = 0; + char *dst; + + if (len == 0) + return NULL; + + if (VIR_ALLOC_N(dst, len + 1) < 0) { + virReportOOMError(); + return NULL; + } for (i = 0; i < len; i++) { switch (src[i]) { @@ -929,16 +978,14 @@ escape_specialcharacters(char *src, char *dst, size_t dstlen) case '\t': continue; default: - temp_buffer[j] = src[i]; + dst[j] = src[i]; j++; } } - temp_buffer[j] = '\0'; - if (virStrcpy(dst, temp_buffer, dstlen) == NULL) - return -1; + dst[j] = '\0'; - return 0; + return dst; } static LIBSSH2_SESSION * @@ -1121,8 +1168,6 @@ phypOpen(virConnectPtr conn, { LIBSSH2_SESSION *session = NULL; ConnectionData *connection_data = NULL; - char *string = NULL; - size_t len = 0; int internal_socket; uuid_tablePtr uuid_table = NULL; phyp_driverPtr phyp_driver = NULL; @@ -1157,13 +1202,6 @@ phypOpen(virConnectPtr conn, } if (conn->uri->path) { - len = strlen(conn->uri->path) + 1; - - if (VIR_ALLOC_N(string, len) < 0) { - virReportOOMError(); - goto failure; - } - /* need to shift one byte in order to remove the first "/" of URI component */ if (conn->uri->path[0] == '/') managed_system = strdup(conn->uri->path + 1); @@ -1183,7 +1221,7 @@ phypOpen(virConnectPtr conn, if (char_ptr) *char_ptr = '\0'; - if (escape_specialcharacters(conn->uri->path, string, len) == -1) { + if (contains_specialcharacters(conn->uri->path)) { PHYP_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Error parsing 'path'. Invalid characters.")); @@ -1242,7 +1280,6 @@ phypOpen(virConnectPtr conn, } VIR_FREE(connection_data); - VIR_FREE(string); return VIR_DRV_OPEN_ERROR; } @@ -1947,15 +1984,10 @@ phypAttachDevice(virDomainPtr domain, const char *xml) virBuffer buf = VIR_BUFFER_INITIALIZER; char *domain_name = NULL; - if (VIR_ALLOC_N(domain_name, sizeof(domain->name)) < 0) { - virReportOOMError(); - goto err; - } + domain_name = escape_specialcharacters(domain->name); - if (escape_specialcharacters - (domain->name, domain_name, strlen(domain->name)) == -1) { - virReportOOMError(); - goto err; + if (domain_name == NULL) { + goto cleanup; } def->os.type = strdup("aix");