From bcd5f12e4bca2ed2c00cddb5ffd046aef6f4fb31 Mon Sep 17 00:00:00 2001 From: David Garcia Quintas Date: Wed, 22 Feb 2017 14:32:56 -0800 Subject: Fixed Heap-buffer-overflow in parse_unix via clusterfuzz --- src/core/ext/client_channel/parse_address.c | 3 ++- src/core/ext/transport/chttp2/client/insecure/channel_create.c | 4 +++- src/core/ext/transport/chttp2/client/secure/secure_channel_create.c | 4 +++- 3 files changed, 8 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/core/ext/client_channel/parse_address.c b/src/core/ext/client_channel/parse_address.c index b1d55ad0f5..fa0125ee9e 100644 --- a/src/core/ext/client_channel/parse_address.c +++ b/src/core/ext/client_channel/parse_address.c @@ -49,9 +49,10 @@ int parse_unix(grpc_uri *uri, grpc_resolved_address *resolved_addr) { struct sockaddr_un *un = (struct sockaddr_un *)resolved_addr->addr; + memset(un, 0, sizeof(*un)); un->sun_family = AF_UNIX; - strcpy(un->sun_path, uri->path); + strncpy(un->sun_path, uri->path, sizeof(un->sun_path) - 1 /* null term'd */); resolved_addr->len = strlen(un->sun_path) + sizeof(un->sun_family) + 1; return 1; diff --git a/src/core/ext/transport/chttp2/client/insecure/channel_create.c b/src/core/ext/transport/chttp2/client/insecure/channel_create.c index 490a0c560e..286232f277 100644 --- a/src/core/ext/transport/chttp2/client/insecure/channel_create.c +++ b/src/core/ext/transport/chttp2/client/insecure/channel_create.c @@ -73,7 +73,9 @@ static grpc_channel *client_channel_factory_create_channel( arg.type = GRPC_ARG_STRING; arg.key = GRPC_ARG_SERVER_URI; arg.value.string = grpc_resolver_factory_add_default_prefix_if_needed(target); - grpc_channel_args *new_args = grpc_channel_args_copy_and_add(args, &arg, 1); + const char *to_remove[] = {GRPC_ARG_SERVER_URI}; + grpc_channel_args *new_args = + grpc_channel_args_copy_and_add_and_remove(args, to_remove, 1, &arg, 1); gpr_free(arg.value.string); grpc_channel *channel = grpc_channel_create(exec_ctx, target, new_args, GRPC_CLIENT_CHANNEL, NULL); diff --git a/src/core/ext/transport/chttp2/client/secure/secure_channel_create.c b/src/core/ext/transport/chttp2/client/secure/secure_channel_create.c index d8c18eb122..825db68c65 100644 --- a/src/core/ext/transport/chttp2/client/secure/secure_channel_create.c +++ b/src/core/ext/transport/chttp2/client/secure/secure_channel_create.c @@ -182,7 +182,9 @@ static grpc_channel *client_channel_factory_create_channel( arg.type = GRPC_ARG_STRING; arg.key = GRPC_ARG_SERVER_URI; arg.value.string = grpc_resolver_factory_add_default_prefix_if_needed(target); - grpc_channel_args *new_args = grpc_channel_args_copy_and_add(args, &arg, 1); + const char *to_remove[] = {GRPC_ARG_SERVER_URI}; + grpc_channel_args *new_args = + grpc_channel_args_copy_and_add_and_remove(args, to_remove, 1, &arg, 1); gpr_free(arg.value.string); grpc_channel *channel = grpc_channel_create(exec_ctx, target, new_args, GRPC_CLIENT_CHANNEL, NULL); -- cgit v1.2.3 From f05359f6975003197d83ec5a1b78893e01000f99 Mon Sep 17 00:00:00 2001 From: David Garcia Quintas Date: Wed, 22 Feb 2017 15:00:41 -0800 Subject: Set terminator explicitly --- src/core/ext/client_channel/parse_address.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/core/ext/client_channel/parse_address.c b/src/core/ext/client_channel/parse_address.c index fa0125ee9e..4b8222824b 100644 --- a/src/core/ext/client_channel/parse_address.c +++ b/src/core/ext/client_channel/parse_address.c @@ -49,10 +49,10 @@ int parse_unix(grpc_uri *uri, grpc_resolved_address *resolved_addr) { struct sockaddr_un *un = (struct sockaddr_un *)resolved_addr->addr; - memset(un, 0, sizeof(*un)); un->sun_family = AF_UNIX; strncpy(un->sun_path, uri->path, sizeof(un->sun_path) - 1 /* null term'd */); + un->sun_path[sizeof(un->sun_path) - 1] = '\0'; resolved_addr->len = strlen(un->sun_path) + sizeof(un->sun_family) + 1; return 1; -- cgit v1.2.3 From e2869fee8db5f7a94858b551089c45bbb1bd943b Mon Sep 17 00:00:00 2001 From: David Garcia Quintas Date: Thu, 23 Feb 2017 09:11:24 -0800 Subject: Simply return 0 on input path too long --- src/core/ext/client_channel/parse_address.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/core/ext/client_channel/parse_address.c b/src/core/ext/client_channel/parse_address.c index 4b8222824b..17cfc9795b 100644 --- a/src/core/ext/client_channel/parse_address.c +++ b/src/core/ext/client_channel/parse_address.c @@ -49,12 +49,12 @@ int parse_unix(grpc_uri *uri, grpc_resolved_address *resolved_addr) { struct sockaddr_un *un = (struct sockaddr_un *)resolved_addr->addr; - + const size_t maxlen = sizeof(un->sun_path); + const size_t path_len = strnlen(uri->path, maxlen); + if (path_len == maxlen) return 0; un->sun_family = AF_UNIX; - strncpy(un->sun_path, uri->path, sizeof(un->sun_path) - 1 /* null term'd */); - un->sun_path[sizeof(un->sun_path) - 1] = '\0'; - resolved_addr->len = strlen(un->sun_path) + sizeof(un->sun_family) + 1; - + strcpy(un->sun_path, uri->path); + resolved_addr->len = path_len + sizeof(un->sun_family) + 1; return 1; } -- cgit v1.2.3 From d9314adb8e3bc687d15269c6e25554741ded4ea6 Mon Sep 17 00:00:00 2001 From: David Garcia Quintas Date: Thu, 23 Feb 2017 09:54:55 -0800 Subject: poopulate ->len with size of struct --- src/core/ext/client_channel/parse_address.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/core/ext/client_channel/parse_address.c b/src/core/ext/client_channel/parse_address.c index 17cfc9795b..8e4da03de0 100644 --- a/src/core/ext/client_channel/parse_address.c +++ b/src/core/ext/client_channel/parse_address.c @@ -54,7 +54,7 @@ int parse_unix(grpc_uri *uri, grpc_resolved_address *resolved_addr) { if (path_len == maxlen) return 0; un->sun_family = AF_UNIX; strcpy(un->sun_path, uri->path); - resolved_addr->len = path_len + sizeof(un->sun_family) + 1; + resolved_addr->len = sizeof(*un); return 1; } -- cgit v1.2.3