Otherwise code like https://code.launchpad.net/~frankban/lpsetup/lp-lxc-ip
can trivially make this code (get_init_pid() in this case) overflow.
This is on top of the patchset I sent yesterday.
Signed-off-by: Serge Hallyn <serge.hal...@ubuntu.com>
---
src/lxc/cgroup.c | 67 +++++++++++++++++++++++++++++++-------
src/lxc/commands.c | 16 ++++++++--
src/lxc/conf.c | 85 +++++++++++++++++++++++++++++++++++++++----------
src/lxc/freezer.c | 6 +++-
src/lxc/lxc_monitor.c | 11 +++++--
src/lxc/network.c | 10 ++++--
src/lxc/state.c | 4 ++-
7 files changed, 164 insertions(+), 35 deletions(-)
diff --git a/src/lxc/cgroup.c b/src/lxc/cgroup.c
index 9af199d..8933c69 100644
--- a/src/lxc/cgroup.c
+++ b/src/lxc/cgroup.c
@@ -176,8 +176,10 @@ static int get_cgroup_mount(const char *subsystem, char
*mnt)
get_init_cgroup(subsystem, NULL,
initcgroup),
(flags & CGROUP_NS_CGROUP) ? "" :
"/lxc");
- if (ret < 0 || ret >= MAXPATHLEN)
+ if (ret < 0 || ret >= MAXPATHLEN) {
+ ERROR("name too long");
goto fail;
+ }
fclose(file);
DEBUG("using cgroup mounted at '%s'", mnt);
return 0;
@@ -207,12 +209,16 @@ static int cgroup_rename_nsgroup(const char *mnt, const
char *name, pid_t pid)
int ret;
ret = snprintf(oldname, MAXPATHLEN, "%s/%d", mnt, pid);
- if (ret >= MAXPATHLEN)
+ if (ret >= MAXPATHLEN) {
+ ERROR("pathname too long");
return -1;
+ }
ret = snprintf(newname, MAXPATHLEN, "%s/%s", mnt, name);
- if (ret >= MAXPATHLEN)
+ if (ret >= MAXPATHLEN) {
+ ERROR("pathname too long");
return -1;
+ }
if (rename(oldname, newname)) {
SYSERROR("failed to rename cgroup %s->%s", oldname, newname);
@@ -250,8 +256,13 @@ int lxc_cgroup_attach(const char *path, pid_t pid)
FILE *f;
char tasks[MAXPATHLEN];
int ret = 0;
+ int rc;
- snprintf(tasks, MAXPATHLEN, "%s/tasks", path);
+ rc = snprintf(tasks, MAXPATHLEN, "%s/tasks", path);
+ if (rc < 0 || rc >= MAXPATHLEN) {
+ ERROR("pathname too long");
+ return -1;
+ }
f = fopen(tasks, "w");
if (!f) {
@@ -445,6 +456,7 @@ int recursive_rmdir(char *dirname)
while (!readdir_r(dir, &dirent, &direntp)) {
struct stat mystat;
+ int rc;
if (!direntp)
break;
@@ -453,7 +465,11 @@ int recursive_rmdir(char *dirname)
!strcmp(direntp->d_name, ".."))
continue;
- snprintf(pathname, MAXPATHLEN, "%s/%s", dirname,
direntp->d_name);
+ rc = snprintf(pathname, MAXPATHLEN, "%s/%s", dirname,
direntp->d_name);
+ if (rc < 0 || rc >= MAXPATHLEN) {
+ ERROR("pathname too long");
+ continue;
+ }
ret = stat(pathname, &mystat);
if (ret)
continue;
@@ -475,10 +491,15 @@ int lxc_one_cgroup_destroy(struct mntent *mntent, const
char *name)
char cgname[MAXPATHLEN], initcgroup[MAXPATHLEN];
char *cgmnt = mntent->mnt_dir;
int flags = get_cgroup_flags(mntent);
+ int rc;
- snprintf(cgname, MAXPATHLEN, "%s%s%s/%s", cgmnt,
+ rc = snprintf(cgname, MAXPATHLEN, "%s%s%s/%s", cgmnt,
get_init_cgroup(NULL, mntent, initcgroup),
(flags & CGROUP_NS_CGROUP) ? "" : "/lxc", name);
+ if (rc < 0 || rc >= MAXPATHLEN) {
+ ERROR("path name too long");
+ return -1;
+ }
DEBUG("destroying %s\n", cgname);
if (recursive_rmdir(cgname)) {
SYSERROR("failed to remove cgroup '%s'", cgname);
@@ -529,11 +550,16 @@ int lxc_cgroup_path_get(char **path, const char
*subsystem, const char *name)
{
static char buf[MAXPATHLEN];
static char retbuf[MAXPATHLEN];
+ int rc;
/* what lxc_cgroup_set calls subsystem is actually the filename, i.e.
'devices.allow'. So for our purposee we trim it */
if (subsystem) {
- snprintf(retbuf, MAXPATHLEN, "%s", subsystem);
+ rc = snprintf(retbuf, MAXPATHLEN, "%s", subsystem);
+ if (rc < 0 || rc >= MAXPATHLEN) {
+ ERROR("subsystem name too long");
+ return -1;
+ }
char *s = index(retbuf, '.');
if (s)
*s = '\0';
@@ -544,7 +570,11 @@ int lxc_cgroup_path_get(char **path, const char
*subsystem, const char *name)
return -1;
}
- snprintf(retbuf, MAXPATHLEN, "%s/%s", buf, name);
+ rc = snprintf(retbuf, MAXPATHLEN, "%s/%s", buf, name);
+ if (rc < 0 || rc >= MAXPATHLEN) {
+ ERROR("name too long");
+ return -1;
+ }
DEBUG("%s: returning %s for subsystem %s", __func__, retbuf, subsystem);
@@ -557,12 +587,17 @@ int lxc_cgroup_set(const char *name, const char
*filename, const char *value)
int fd, ret;
char *dirpath;
char path[MAXPATHLEN];
+ int rc;
ret = lxc_cgroup_path_get(&dirpath, filename, name);
if (ret)
return -1;
- snprintf(path, MAXPATHLEN, "%s/%s", dirpath, filename);
+ rc = snprintf(path, MAXPATHLEN, "%s/%s", dirpath, filename);
+ if (rc < 0 || rc >= MAXPATHLEN) {
+ ERROR("pathname too long");
+ return -1;
+ }
fd = open(path, O_WRONLY);
if (fd < 0) {
@@ -588,12 +623,17 @@ int lxc_cgroup_get(const char *name, const char *filename,
int fd, ret = -1;
char *dirpath;
char path[MAXPATHLEN];
+ int rc;
ret = lxc_cgroup_path_get(&dirpath, filename, name);
if (ret)
return -1;
- snprintf(path, MAXPATHLEN, "%s/%s", dirpath, filename);
+ rc = snprintf(path, MAXPATHLEN, "%s/%s", dirpath, filename);
+ if (rc < 0 || rc >= MAXPATHLEN) {
+ ERROR("pathname too long");
+ return -1;
+ }
fd = open(path, O_RDONLY);
if (fd < 0) {
@@ -615,12 +655,17 @@ int lxc_cgroup_nrtasks(const char *name)
char path[MAXPATHLEN];
int pid, ret, count = 0;
FILE *file;
+ int rc;
ret = lxc_cgroup_path_get(&dpath, NULL, name);
if (ret)
return -1;
- snprintf(path, MAXPATHLEN, "%s/tasks", dpath);
+ rc = snprintf(path, MAXPATHLEN, "%s/tasks", dpath);
+ if (rc < 0 || rc >= MAXPATHLEN) {
+ ERROR("pathname too long");
+ return -1;
+ }
file = fopen(path, "r");
if (!file) {
diff --git a/src/lxc/commands.c b/src/lxc/commands.c
index 1d488ae..cce24db 100644
--- a/src/lxc/commands.c
+++ b/src/lxc/commands.c
@@ -75,8 +75,14 @@ static int __lxc_command(const char *name, struct
lxc_command *command,
int sock, ret = -1;
char path[sizeof(((struct sockaddr_un *)0)->sun_path)] = { 0 };
char *offset = &path[1];
+ int rc, len;
- sprintf(offset, abstractname, name);
+ len = sizeof(path)-1;
+ rc = snprintf(offset, len, abstractname, name);
+ if (rc < 0 || rc >= len) {
+ ERROR("Name too long");
+ return -1;
+ }
sock = lxc_af_unix_connect(path);
if (sock < 0 && errno == ECONNREFUSED) {
@@ -266,8 +272,14 @@ extern int lxc_command_mainloop_add(const char *name,
int ret, fd;
char path[sizeof(((struct sockaddr_un *)0)->sun_path)] = { 0 };
char *offset = &path[1];
+ int rc, len;
- sprintf(offset, abstractname, name);
+ len = sizeof(path)-1;
+ rc = snprintf(offset, len, abstractname, name);
+ if (rc < 0 || rc >= len) {
+ ERROR("Name too long");
+ return -1;
+ }
fd = lxc_af_unix_open(path, SOCK_STREAM, 0);
if (fd < 0) {
diff --git a/src/lxc/conf.c b/src/lxc/conf.c
index 87f7adc..9cc5f6b 100644
--- a/src/lxc/conf.c
+++ b/src/lxc/conf.c
@@ -241,11 +241,25 @@ static int run_script(const char *name, const char
*section,
return -1;
}
- ret = sprintf(buffer, "%s %s %s", script, name, section);
+ ret = snprintf(buffer, size, "%s %s %s", script, name, section);
+ if (ret < 0 || ret >= size) {
+ ERROR("Script name too long");
+ free(buffer);
+ return -1;
+ }
va_start(ap, script);
- while ((p = va_arg(ap, char *)))
- ret += sprintf(buffer + ret, " %s", p);
+ while ((p = va_arg(ap, char *))) {
+ int len = size-ret;
+ int rc;
+ rc = snprintf(buffer + ret, len, " %s", p);
+ if (rc < 0 || rc >= len) {
+ free(buffer);
+ ERROR("Script args too long");
+ return -1;
+ }
+ ret += rc;
+ }
va_end(ap);
f = popen(buffer, "r");
@@ -391,7 +405,7 @@ static int mount_rootfs_file(const char *rootfs, const char
*target)
{
struct dirent dirent, *direntp;
struct loop_info64 loinfo;
- int ret = -1, fd = -1;
+ int ret = -1, fd = -1, rc;
DIR *dir;
char path[MAXPATHLEN];
@@ -415,7 +429,10 @@ static int mount_rootfs_file(const char *rootfs, const
char *target)
if (strncmp(direntp->d_name, "loop", 4))
continue;
- sprintf(path, "/dev/%s", direntp->d_name);
+ rc = snprintf(path, MAXPATHLEN, "/dev/%s", direntp->d_name);
+ if (rc < 0 || rc >= MAXPATHLEN)
+ continue;
+
fd = open(path, O_RDWR);
if (fd < 0)
continue;
@@ -581,7 +598,7 @@ static int setup_tty(const struct lxc_rootfs *rootfs,
}
if (ttydir) {
/* create dev/lxc/tty%d" */
- snprintf(lxcpath, sizeof(lxcpath), "%s/dev/%s/tty%d",
+ ret = snprintf(lxcpath, sizeof(lxcpath),
"%s/dev/%s/tty%d",
rootfs->mount, ttydir, i + 1);
if (ret >= sizeof(lxcpath)) {
ERROR("pathname too long for ttys");
@@ -605,7 +622,11 @@ static int setup_tty(const struct lxc_rootfs *rootfs,
continue;
}
- snprintf(lxcpath, sizeof(lxcpath), "%s/tty%d", ttydir,
i+1);
+ ret = snprintf(lxcpath, sizeof(lxcpath), "%s/tty%d",
ttydir, i+1);
+ if (ret >= sizeof(lxcpath)) {
+ ERROR("tty pathname too long");
+ return -1;
+ }
ret = symlink(lxcpath, path);
if (ret) {
SYSERROR("failed to create symlink for tty
%d\n", i+1);
@@ -686,12 +707,17 @@ static int umount_oldrootfs(const char *oldrootfs)
void *cbparm[2];
struct lxc_list mountlist, *iterator;
int ok, still_mounted, last_still_mounted;
+ int rc;
/* read and parse /proc/mounts in old root fs */
lxc_list_init(&mountlist);
/* oldrootfs is on the top tree directory now */
- snprintf(path, sizeof(path), "/%s", oldrootfs);
+ rc = snprintf(path, sizeof(path), "/%s", oldrootfs);
+ if (rc >= sizeof(path)) {
+ ERROR("rootfs name too long");
+ return -1;
+ }
cbparm[0] = &mountlist;
cbparm[1] = strdup(path);
@@ -700,7 +726,11 @@ static int umount_oldrootfs(const char *oldrootfs)
return -1;
}
- snprintf(path, sizeof(path), "%s/proc/mounts", oldrootfs);
+ rc = snprintf(path, sizeof(path), "%s/proc/mounts", oldrootfs);
+ if (rc >= sizeof(path)) {
+ ERROR("container proc/mounts name too long");
+ return -1;
+ }
ok = lxc_file_for_each_line(path,
setup_rootfs_pivot_root_cb, &cbparm);
@@ -754,6 +784,7 @@ static int setup_rootfs_pivot_root(const char *rootfs,
const char *pivotdir)
{
char path[MAXPATHLEN];
int remove_pivotdir = 0;
+ int rc;
/* change into new root fs */
if (chdir(rootfs)) {
@@ -765,7 +796,11 @@ static int setup_rootfs_pivot_root(const char *rootfs,
const char *pivotdir)
pivotdir = "mnt";
/* compute the full path to pivotdir under rootfs */
- snprintf(path, sizeof(path), "%s/%s", rootfs, pivotdir);
+ rc = snprintf(path, sizeof(path), "%s/%s", rootfs, pivotdir);
+ if (rc >= sizeof(path)) {
+ ERROR("pivot dir name too long");
+ return -1;
+ }
if (access(path, F_OK)) {
@@ -988,7 +1023,11 @@ static int setup_ttydir_console(const struct lxc_rootfs
*rootfs,
}
/* create symlink from rootfs/dev/console to 'lxc/console' */
- snprintf(lxcpath, sizeof(lxcpath), "%s/console", ttydir);
+ ret = snprintf(lxcpath, sizeof(lxcpath), "%s/console", ttydir);
+ if (ret >= sizeof(lxcpath)) {
+ ERROR("lxc/console path too long");
+ return -1;
+ }
ret = symlink(lxcpath, path);
if (ret) {
SYSERROR("failed to create symlink for console");
@@ -1182,7 +1221,7 @@ skipvarlib:
skipabs:
- snprintf(path, MAXPATHLEN, "%s/%s", rootfs->mount,
+ r = snprintf(path, MAXPATHLEN, "%s/%s", rootfs->mount,
aux + offset);
if (r < 0 || r >= MAXPATHLEN) {
WARN("pathnme too long for '%s'", mntent->mnt_dir);
@@ -1213,7 +1252,11 @@ static int mount_entry_on_relative_rootfs(struct mntent
*mntent,
}
/* relative to root mount point */
- snprintf(path, sizeof(path), "%s/%s", rootfs, mntent->mnt_dir);
+ ret = snprintf(path, sizeof(path), "%s/%s", rootfs, mntent->mnt_dir);
+ if (ret >= sizeof(path)) {
+ ERROR("path name too long");
+ return -1;
+ }
ret = mount_entry(mntent->mnt_fsname, path, mntent->mnt_type,
mntflags, mntdata);
@@ -1679,7 +1722,11 @@ static int instanciate_veth(struct lxc_handler *handler,
struct lxc_netdev *netd
if (netdev->priv.veth_attr.pair)
veth1 = netdev->priv.veth_attr.pair;
else {
- snprintf(veth1buf, sizeof(veth1buf), "vethXXXXXX");
+ err = snprintf(veth1buf, sizeof(veth1buf), "vethXXXXXX");
+ if (err >= sizeof(veth1buf)) { /* can't *really* happen, but...
*/
+ ERROR("veth1 name too long");
+ return -1;
+ }
veth1 = mktemp(veth1buf);
}
@@ -1767,7 +1814,9 @@ static int instanciate_macvlan(struct lxc_handler
*handler, struct lxc_netdev *n
return -1;
}
- snprintf(peerbuf, sizeof(peerbuf), "mcXXXXXX");
+ err = snprintf(peerbuf, sizeof(peerbuf), "mcXXXXXX");
+ if (err >= sizeof(peerbuf))
+ return -1;
peer = mktemp(peerbuf);
if (!strlen(peer)) {
@@ -1814,7 +1863,11 @@ static int instanciate_vlan(struct lxc_handler *handler,
struct lxc_netdev *netd
return -1;
}
- snprintf(peer, sizeof(peer), "vlan%d", netdev->priv.vlan_attr.vid);
+ err = snprintf(peer, sizeof(peer), "vlan%d",
netdev->priv.vlan_attr.vid);
+ if (err >= sizeof(peer)) {
+ ERROR("peer name too long");
+ return -1;
+ }
err = lxc_vlan_create(netdev->link, peer, netdev->priv.vlan_attr.vid);
if (err) {
diff --git a/src/lxc/freezer.c b/src/lxc/freezer.c
index 94984a3..2a6f0f5 100644
--- a/src/lxc/freezer.c
+++ b/src/lxc/freezer.c
@@ -49,7 +49,11 @@ static int freeze_unfreeze(const char *name, int freeze)
if (ret)
return -1;
- snprintf(freezer, MAXPATHLEN, "%s/freezer.state", nsgroup);
+ ret = snprintf(freezer, MAXPATHLEN, "%s/freezer.state", nsgroup);
+ if (ret >= MAXPATHLEN) {
+ ERROR("freezer.state name too long");
+ return -1;
+ }
fd = open(freezer, O_RDWR);
if (fd < 0) {
diff --git a/src/lxc/lxc_monitor.c b/src/lxc/lxc_monitor.c
index 3802d2e..10168fb 100644
--- a/src/lxc/lxc_monitor.c
+++ b/src/lxc/lxc_monitor.c
@@ -60,6 +60,7 @@ int main(int argc, char *argv[])
struct lxc_msg msg;
regex_t preg;
int fd;
+ int len, rc;
if (lxc_arguments_parse(&my_args, argc, argv))
return -1;
@@ -68,12 +69,18 @@ int main(int argc, char *argv[])
my_args.progname, my_args.quiet))
return -1;
- regexp = malloc(strlen(my_args.name) + 3);
+ len = strlen(my_args.name) + 3;
+ regexp = malloc(len + 3);
if (!regexp) {
ERROR("failed to allocate memory");
return -1;
}
- sprintf(regexp, "^%s$", my_args.name);
+ rc = snprintf(regexp, len, "^%s$", my_args.name);
+ if (rc < 0 || rc >= len) {
+ ERROR("Name too long");
+ free(regexp);
+ return -1;
+ }
if (regcomp(&preg, regexp, REG_NOSUB|REG_EXTENDED)) {
ERROR("failed to compile the regex '%s'", my_args.name);
diff --git a/src/lxc/network.c b/src/lxc/network.c
index 214460b..c8aecfb 100644
--- a/src/lxc/network.c
+++ b/src/lxc/network.c
@@ -582,12 +582,15 @@ static int proc_sys_net_write(const char *path, const
char *value)
static int ip_forward_set(const char *ifname, int family, int flag)
{
char path[MAXPATHLEN];
+ int rc;
if (family != AF_INET && family != AF_INET6)
return -EINVAL;
- snprintf(path, MAXPATHLEN, "/proc/sys/net/%s/conf/%s/forwarding",
+ rc = snprintf(path, MAXPATHLEN, "/proc/sys/net/%s/conf/%s/forwarding",
family == AF_INET?"ipv4":"ipv6" , ifname);
+ if (rc >= MAXPATHLEN)
+ return -E2BIG;
return proc_sys_net_write(path, flag?"1":"0");
}
@@ -605,13 +608,16 @@ int lxc_ip_forward_off(const char *ifname, int family)
static int neigh_proxy_set(const char *ifname, int family, int flag)
{
char path[MAXPATHLEN];
+ int ret;
if (family != AF_INET && family != AF_INET6)
return -EINVAL;
- sprintf(path, "/proc/sys/net/%s/conf/%s/%s",
+ ret = snprintf(path, MAXPATHLEN, "/proc/sys/net/%s/conf/%s/%s",
family == AF_INET?"ipv4":"ipv6" , ifname,
family == AF_INET?"proxy_arp":"proxy_ndp");
+ if (ret < 0 || ret >= MAXPATHLEN)
+ return -E2BIG;
return proc_sys_net_write(path, flag?"1":"0");
}
diff --git a/src/lxc/state.c b/src/lxc/state.c
index b435eba..9983656 100644
--- a/src/lxc/state.c
+++ b/src/lxc/state.c
@@ -75,7 +75,9 @@ static int freezer_state(const char *name)
if (err)
return -1;
- snprintf(freezer, MAXPATHLEN, "%s/freezer.state", nsgroup);
+ err = snprintf(freezer, MAXPATHLEN, "%s/freezer.state", nsgroup);
+ if (err < 0 || err >= MAXPATHLEN)
+ return -1;
file = fopen(freezer, "r");
if (!file)
--
1.7.9.5
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Lxc-users mailing list
Lxc-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/lxc-users
