From c56ac33b7a7bee960dd43f7744aaca47886c3960 Mon Sep 17 00:00:00 2001 From: Prasanna Kumar Kalever Date: Tue, 9 Aug 2016 14:20:09 +0530 Subject: [PATCH] block/gluster: improve defense over string to int conversion using atoi() for converting string to int may be error prone in case if string supplied in the argument is not a fold of numerical number, This is not a bug because in the existing code, static QemuOptsList runtime_tcp_opts = { .name = "gluster_tcp", .head = QTAILQ_HEAD_INITIALIZER(runtime_tcp_opts.head), .desc = { ... { .name = GLUSTER_OPT_PORT, .type = QEMU_OPT_NUMBER, .help = "port number ...", }, ... }; port type is QEMU_OPT_NUMBER, before we actually reaches atoi() port is already defended by parse_option_number() However It is a good practice to use function like parse_uint_full() over atoi() to keep port self defended Note: As now the port string to int conversion has its defence code set, and also we understand that port argument is actually a string type, in the follow up patch let's move port type from QEMU_OPT_NUMBER to QEMU_OPT_STRING [Jeff Cody: removed spurious parenthesis] Signed-off-by: Prasanna Kumar Kalever Reviewed-by: Markus Armbruster Signed-off-by: Jeff Cody --- block/gluster.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/block/gluster.c b/block/gluster.c index 40bd29c781..98a91323b6 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -14,6 +14,7 @@ #include "qapi/qmp/qerror.h" #include "qemu/uri.h" #include "qemu/error-report.h" +#include "qemu/cutils.h" #define GLUSTER_OPT_FILENAME "filename" #define GLUSTER_OPT_VOLUME "volume" @@ -394,6 +395,7 @@ static struct glfs *qemu_gluster_glfs_init(BlockdevOptionsGluster *gconf, int ret; int old_errno; GlusterServerList *server; + unsigned long long port; glfs = glfs_find_preopened(gconf->volume); if (glfs) { @@ -413,10 +415,17 @@ static struct glfs *qemu_gluster_glfs_init(BlockdevOptionsGluster *gconf, GlusterTransport_lookup[server->value->type], server->value->u.q_unix.path, 0); } else { + if (parse_uint_full(server->value->u.tcp.port, &port, 10) < 0 || + port > 65535) { + error_setg(errp, "'%s' is not a valid port number", + server->value->u.tcp.port); + errno = EINVAL; + goto out; + } ret = glfs_set_volfile_server(glfs, GlusterTransport_lookup[server->value->type], server->value->u.tcp.host, - atoi(server->value->u.tcp.port)); + (int)port); } if (ret < 0) {