From mboxrd@z Thu Jan 1 00:00:00 1970 From: "G. Campana" Subject: [PATCH 2/5] kvmtool: 9p: fix sprintf vulnerabilities Date: Thu, 10 Nov 2016 16:21:08 +0100 Message-ID: <1478791271-7558-3-git-send-email-gcampana+kvm@quarkslab.com> References: <1478791271-7558-1-git-send-email-gcampana+kvm@quarkslab.com> Cc: kvm@vger.kernel.org, andre.przywara@arm.com, gcampana+kvm@quarkslab.com To: Will.Deacon@arm.com Return-path: Received: from mail.quarkslab.com ([195.154.215.101]:51350 "EHLO mail.quarkslab.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933776AbcKJPWN (ORCPT ); Thu, 10 Nov 2016 10:22:13 -0500 In-Reply-To: <1478791271-7558-1-git-send-email-gcampana+kvm@quarkslab.com> Sender: kvm-owner@vger.kernel.org List-ID: Use snprintf instead of sprintf to avoid buffer overflow vulnerabilities. Signed-off-by: G. Campana --- virtio/9p.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 70 insertions(+), 11 deletions(-) diff --git a/virtio/9p.c b/virtio/9p.c index c3edc20..4116252 100644 --- a/virtio/9p.c +++ b/virtio/9p.c @@ -280,6 +280,7 @@ static void virtio_p9_create(struct p9_dev *p9dev, { int fd, ret; char *name; + size_t size; struct stat st; struct p9_qid qid; struct p9_fid *dfid; @@ -292,12 +293,26 @@ static void virtio_p9_create(struct p9_dev *p9dev, flags = virtio_p9_openflags(flags); - sprintf(full_path, "%s/%s", dfid->abs_path, name); + ret = snprintf(full_path, sizeof(full_path), "%s/%s", dfid->abs_path, name); + if (ret >= (int)sizeof(full_path)) { + errno = ENAMETOOLONG; + goto err_out; + } + if (path_is_illegal(full_path)) { errno = EACCES; goto err_out; } + size = sizeof(dfid->abs_path) - (dfid->path - dfid->abs_path); + ret = snprintf(dfid->path, size, "%s/%s", dfid->path, name); + if (ret >= (int)size) { + errno = ENAMETOOLONG; + if (size > 0) + dfid->path[size] = '\x00'; + goto err_out; + } + fd = open(full_path, flags | O_CREAT, mode); if (fd < 0) goto err_out; @@ -310,7 +325,6 @@ static void virtio_p9_create(struct p9_dev *p9dev, if (ret < 0) goto err_out; - sprintf(dfid->path, "%s/%s", dfid->path, name); stat2qid(&st, &qid); virtio_p9_pdu_writef(pdu, "Qd", &qid, 0); *outlen = pdu->write_offset; @@ -338,7 +352,12 @@ static void virtio_p9_mkdir(struct p9_dev *p9dev, &name, &mode, &gid); dfid = get_fid(p9dev, dfid_val); - sprintf(full_path, "%s/%s", dfid->abs_path, name); + ret = snprintf(full_path, sizeof(full_path), "%s/%s", dfid->abs_path, name); + if (ret >= (int)sizeof(full_path)) { + errno = ENAMETOOLONG; + goto err_out; + } + if (path_is_illegal(full_path)) { errno = EACCES; goto err_out; @@ -393,11 +412,16 @@ static void virtio_p9_walk(struct p9_dev *p9dev, char tmp[PATH_MAX] = {0}; char full_path[PATH_MAX]; char *str; + int ret; virtio_p9_pdu_readf(pdu, "s", &str); /* Format the new path we're 'walk'ing into */ - sprintf(tmp, "%s/%s", new_fid->path, str); + ret = snprintf(tmp, sizeof(tmp), "%s/%s", new_fid->path, str); + if (ret >= (int)sizeof(tmp)) { + errno = ENAMETOOLONG; + goto err_out; + } free(str); @@ -800,7 +824,12 @@ static void virtio_p9_rename(struct p9_dev *p9dev, fid = get_fid(p9dev, fid_val); new_fid = get_fid(p9dev, new_fid_val); - sprintf(full_path, "%s/%s", new_fid->abs_path, new_name); + ret = snprintf(full_path, sizeof(full_path), "%s/%s", new_fid->abs_path, new_name); + if (ret >= (int)sizeof(full_path)) { + errno = ENAMETOOLONG; + goto err_out; + } + if (path_is_illegal(full_path)) { errno = EACCES; goto err_out; @@ -889,7 +918,12 @@ static void virtio_p9_mknod(struct p9_dev *p9dev, &major, &minor, &gid); dfid = get_fid(p9dev, fid_val); - sprintf(full_path, "%s/%s", dfid->abs_path, name); + ret = snprintf(full_path, sizeof(full_path), "%s/%s", dfid->abs_path, name); + if (ret >= (int)sizeof(full_path)) { + errno = ENAMETOOLONG; + goto err_out; + } + if (path_is_illegal(full_path)) { errno = EACCES; goto err_out; @@ -961,7 +995,12 @@ static void virtio_p9_symlink(struct p9_dev *p9dev, virtio_p9_pdu_readf(pdu, "dssd", &fid_val, &name, &old_path, &gid); dfid = get_fid(p9dev, fid_val); - sprintf(new_name, "%s/%s", dfid->abs_path, name); + ret = snprintf(new_name, sizeof(new_name), "%s/%s", dfid->abs_path, name); + if (ret >= (int)sizeof(new_name)) { + errno = ENAMETOOLONG; + goto err_out; + } + if (path_is_illegal(new_name)) { errno = EACCES; goto err_out; @@ -1001,7 +1040,12 @@ static void virtio_p9_link(struct p9_dev *p9dev, dfid = get_fid(p9dev, dfid_val); fid = get_fid(p9dev, fid_val); - sprintf(full_path, "%s/%s", dfid->abs_path, name); + ret = snprintf(full_path, sizeof(full_path), "%s/%s", dfid->abs_path, name); + if (ret >= (int)sizeof(full_path)) { + errno = ENAMETOOLONG; + goto err_out; + } + if (path_is_illegal(full_path)) { errno = EACCES; goto err_out; @@ -1122,8 +1166,18 @@ static void virtio_p9_renameat(struct p9_dev *p9dev, old_dfid = get_fid(p9dev, old_dfid_val); new_dfid = get_fid(p9dev, new_dfid_val); - sprintf(old_full_path, "%s/%s", old_dfid->abs_path, old_name); - sprintf(new_full_path, "%s/%s", new_dfid->abs_path, new_name); + ret = snprintf(old_full_path, sizeof(old_full_path), "%s/%s", old_dfid->abs_path, old_name); + if (ret >= (int)sizeof(old_full_path)) { + errno = ENAMETOOLONG; + goto err_out; + } + + ret = snprintf(new_full_path, sizeof(new_full_path), "%s/%s", new_dfid->abs_path, new_name); + if (ret >= (int)sizeof(new_full_path)) { + errno = ENAMETOOLONG; + goto err_out; + } + if (path_is_illegal(old_full_path) || path_is_illegal(new_full_path)) { errno = EACCES; goto err_out; @@ -1161,7 +1215,12 @@ static void virtio_p9_unlinkat(struct p9_dev *p9dev, virtio_p9_pdu_readf(pdu, "dsd", &fid_val, &name, &flags); fid = get_fid(p9dev, fid_val); - sprintf(full_path, "%s/%s", fid->abs_path, name); + ret = snprintf(full_path, sizeof(full_path), "%s/%s", fid->abs_path, name); + if (ret >= (int)sizeof(full_path)) { + errno = ENAMETOOLONG; + goto err_out; + } + if (path_is_illegal(full_path)) { errno = EACCES; goto err_out; -- 2.7.4