Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
SUSE:SLE-12-SP3:GA
xen.950
CVE-2015-3259-xsa137.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File CVE-2015-3259-xsa137.patch of Package xen.950
References: bnc#935634 CVE-2015-3259 XSA-137 From 593fe52faa1b85567a7ec20c69d8cfbc7368ae5b Mon Sep 17 00:00:00 2001 From: Ian Jackson <ian.jackson@eu.citrix.com> Date: Mon, 15 Jun 2015 14:50:42 +0100 Subject: [PATCH] xl: Sane handling of extra config file arguments Various xl sub-commands take additional parameters containing = as additional config fragments. The handling of these config fragments has a number of bugs: 1. Use of a static 1024-byte buffer. (If truncation would occur, with semi-trusted input, a security risk arises due to quotes being lost.) 2. Mishandling of the return value from snprintf, so that if truncation occurs, the to-write pointer is updated with the wanted-to-write length, resulting in stack corruption. (This is XSA-137.) 3. Clone-and-hack of the code for constructing the appended config file. These are fixed here, by introducing a new function `string_realloc_append' and using it everywhere. The `extra_info' buffers are replaced by pointers, which start off NULL and are explicitly freed on all return paths. The separate variable which will become dom_info.extra_config is abolished (which involves moving the clearing of dom_info). Additional bugs I observe, not fixed here: 4. The functions which now call string_realloc_append use ad-hoc error returns, with multiple calls to `return'. This currently necessitates multiple new calls to `free'. 5. Many of the paths in xl call exit(-rc) where rc is a libxl status code. This is a ridiculous exit status `convention'. 6. The loops for handling extra config data are clone-and-hacks. 7. Once the extra config buffer is accumulated, it must be combined with the appropriate main config file. The code to do this combining is clone-and-hacked too. Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> Tested-by: Ian Jackson <Ian.Jackson@eu.citrix.com> Acked-by: Ian Campbell <ian,campbell@citrix.com> --- v2: Use SSIZE_MAX, not INT_MAX. Check *accumulate for NULL, not accumulate. Move memset of dom_info. --- tools/libxl/xl_cmdimpl.c | 64 +++++++++++++++++++++++++++++----------------- 1 file changed, 40 insertions(+), 24 deletions(-) Index: xen-4.4.2-testing/tools/libxl/xl_cmdimpl.c =================================================================== --- xen-4.4.2-testing.orig/tools/libxl/xl_cmdimpl.c +++ xen-4.4.2-testing/tools/libxl/xl_cmdimpl.c @@ -151,7 +151,7 @@ struct domain_create { int console_autoconnect; int checkpointed_stream; const char *config_file; - const char *extra_config; /* extra config string */ + char *extra_config; /* extra config string */ const char *restore_file; int migrate_fd; /* -1 means none */ char **migration_domname_r; /* from malloc */ @@ -4332,11 +4332,25 @@ int main_vm_list(int argc, char **argv) return 0; } +static void string_realloc_append(char **accumulate, const char *more) +{ + /* Appends more to accumulate. Accumulate is either NULL, or + * points (always) to a malloc'd nul-terminated string. */ + + size_t oldlen = *accumulate ? strlen(*accumulate) : 0; + size_t morelen = strlen(more) + 1/*nul*/; + if (oldlen > SSIZE_MAX || morelen > SSIZE_MAX - oldlen) { + fprintf(stderr,"Additional config data far too large\n"); + exit(-ERROR_FAIL); + } + + *accumulate = xrealloc(*accumulate, oldlen + morelen); + memcpy(*accumulate + oldlen, more, morelen); +} + int main_create(int argc, char **argv) { const char *filename = NULL; - char *p; - char extra_config[1024]; struct domain_create dom_info; int paused = 0, debug = 0, daemonize = 1, console_autoconnect = 0, quiet = 0, monitor = 1, vnc = 0, vncautopass = 0; @@ -4351,6 +4365,8 @@ int main_create(int argc, char **argv) {0, 0, 0, 0} }; + dom_info.extra_config = NULL; + if (argv[1] && argv[1][0] != '-' && !strchr(argv[1], '=')) { filename = argv[1]; argc--; argv++; @@ -4390,20 +4406,21 @@ int main_create(int argc, char **argv) break; } - extra_config[0] = '\0'; - for (p = extra_config; optind < argc; optind++) { + memset(&dom_info, 0, sizeof(dom_info)); + + for (; optind < argc; optind++) { if (strchr(argv[optind], '=') != NULL) { - p += snprintf(p, sizeof(extra_config) - (p - extra_config), - "%s\n", argv[optind]); + string_realloc_append(&dom_info.extra_config, argv[optind]); + string_realloc_append(&dom_info.extra_config, "\n"); } else if (!filename) { filename = argv[optind]; } else { help("create"); + free(dom_info.extra_config); return 2; } } - memset(&dom_info, 0, sizeof(dom_info)); dom_info.debug = debug; dom_info.daemonize = daemonize; dom_info.monitor = monitor; @@ -4411,16 +4428,18 @@ int main_create(int argc, char **argv) dom_info.dryrun = dryrun_only; dom_info.quiet = quiet; dom_info.config_file = filename; - dom_info.extra_config = extra_config; dom_info.migrate_fd = -1; dom_info.vnc = vnc; dom_info.vncautopass = vncautopass; dom_info.console_autoconnect = console_autoconnect; rc = create_domain(&dom_info); - if (rc < 0) + if (rc < 0) { + free(dom_info.extra_config); return -rc; + } + free(dom_info.extra_config); return 0; } @@ -4428,8 +4447,7 @@ int main_config_update(int argc, char ** { uint32_t domid; const char *filename = NULL; - char *p; - char extra_config[1024]; + char *extra_config = NULL; void *config_data = 0; int config_len = 0; libxl_domain_config d_config; @@ -4464,15 +4482,15 @@ int main_config_update(int argc, char ** break; } - extra_config[0] = '\0'; - for (p = extra_config; optind < argc; optind++) { + for (; optind < argc; optind++) { if (strchr(argv[optind], '=') != NULL) { - p += snprintf(p, sizeof(extra_config) - (p - extra_config), - "%s\n", argv[optind]); + string_realloc_append(&extra_config, argv[optind]); + string_realloc_append(&extra_config, "\n"); } else if (!filename) { filename = argv[optind]; } else { help("create"); + free(extra_config); return 2; } } @@ -4481,7 +4499,8 @@ int main_config_update(int argc, char ** rc = libxl_read_file_contents(ctx, filename, &config_data, &config_len); if (rc) { fprintf(stderr, "Failed to read config file: %s: %s\n", - filename, strerror(errno)); return ERROR_FAIL; } + filename, strerror(errno)); + free(extra_config); return ERROR_FAIL; } if (strlen(extra_config)) { if (config_len > INT_MAX - (strlen(extra_config) + 2 + 1)) { fprintf(stderr, "Failed to attach extra configration\n"); @@ -4522,7 +4541,7 @@ int main_config_update(int argc, char ** libxl_domain_config_dispose(&d_config); free(config_data); - + free(extra_config); return 0; } @@ -6571,7 +6590,7 @@ int main_cpupoolcreate(int argc, char ** { const char *filename = NULL, *config_src=NULL; const char *p; - char extra_config[1024]; + char *extra_config = NULL; int opt; static struct option opts[] = { {"defconfig", 1, 0, 'f'}, @@ -6605,13 +6624,10 @@ int main_cpupoolcreate(int argc, char ** break; } - memset(extra_config, 0, sizeof(extra_config)); while (optind < argc) { if ((p = strchr(argv[optind], '='))) { - if (strlen(extra_config) + 1 + strlen(argv[optind]) < sizeof(extra_config)) { - strcat(extra_config, "\n"); - strcat(extra_config, argv[optind]); - } + string_realloc_append(&extra_config, "\n"); + string_realloc_append(&extra_config, argv[optind]); } else if (!filename) { filename = argv[optind]; } else {
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
