Skip to content

Commit 5c3fcae

Browse files
committed
CVE-2015-1334: Don't use the container's /proc during attach
A user could otherwise over-mount /proc and prevent the apparmor profile or selinux label from being written which combined with a modified /bin/sh or other commonly used binary would lead to unconfined code execution. Reported-by: Roman Fiedler Signed-off-by: Stéphane Graber <stgraber@ubuntu.com>
1 parent 72cf81f commit 5c3fcae

File tree

1 file changed

+93
-13
lines changed

1 file changed

+93
-13
lines changed

Diff for: src/lxc/attach.c

+93-13
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,82 @@
7676

7777
lxc_log_define(lxc_attach, lxc);
7878

79+
int lsm_set_label_at(int procfd, int on_exec, char* lsm_label) {
80+
int labelfd = -1;
81+
int ret = 0;
82+
const char* name;
83+
char* command = NULL;
84+
85+
name = lsm_name();
86+
87+
if (strcmp(name, "nop") == 0)
88+
goto out;
89+
90+
if (strcmp(name, "none") == 0)
91+
goto out;
92+
93+
/* We don't support on-exec with AppArmor */
94+
if (strcmp(name, "AppArmor") == 0)
95+
on_exec = 0;
96+
97+
if (on_exec) {
98+
labelfd = openat(procfd, "self/attr/exec", O_RDWR);
99+
}
100+
else {
101+
labelfd = openat(procfd, "self/attr/current", O_RDWR);
102+
}
103+
104+
if (labelfd < 0) {
105+
SYSERROR("Unable to open LSM label");
106+
ret = -1;
107+
goto out;
108+
}
109+
110+
if (strcmp(name, "AppArmor") == 0) {
111+
int size;
112+
113+
command = malloc(strlen(lsm_label) + strlen("changeprofile ") + 1);
114+
if (!command) {
115+
SYSERROR("Failed to write apparmor profile");
116+
ret = -1;
117+
goto out;
118+
}
119+
120+
size = sprintf(command, "changeprofile %s", lsm_label);
121+
if (size < 0) {
122+
SYSERROR("Failed to write apparmor profile");
123+
ret = -1;
124+
goto out;
125+
}
126+
127+
if (write(labelfd, command, size + 1) < 0) {
128+
SYSERROR("Unable to set LSM label");
129+
ret = -1;
130+
goto out;
131+
}
132+
}
133+
else if (strcmp(name, "SELinux") == 0) {
134+
if (write(labelfd, lsm_label, strlen(lsm_label) + 1) < 0) {
135+
SYSERROR("Unable to set LSM label");
136+
ret = -1;
137+
goto out;
138+
}
139+
}
140+
else {
141+
ERROR("Unable to restore label for unknown LSM: %s", name);
142+
ret = -1;
143+
goto out;
144+
}
145+
146+
out:
147+
free(command);
148+
149+
if (labelfd != -1)
150+
close(labelfd);
151+
152+
return ret;
153+
}
154+
79155
static struct lxc_proc_context_info *lxc_proc_get_context_info(pid_t pid)
80156
{
81157
struct lxc_proc_context_info *info = calloc(1, sizeof(*info));
@@ -570,6 +646,7 @@ struct attach_clone_payload {
570646
struct lxc_proc_context_info* init_ctx;
571647
lxc_attach_exec_t exec_function;
572648
void* exec_payload;
649+
int procfd;
573650
};
574651

575652
static int attach_child_main(void* data);
@@ -622,6 +699,7 @@ int lxc_attach(const char* name, const char* lxcpath, lxc_attach_exec_t exec_fun
622699
char* cwd;
623700
char* new_cwd;
624701
int ipc_sockets[2];
702+
int procfd;
625703
signed long personality;
626704

627705
if (!options)
@@ -833,6 +911,13 @@ int lxc_attach(const char* name, const char* lxcpath, lxc_attach_exec_t exec_fun
833911
rexit(-1);
834912
}
835913

914+
procfd = open("/proc", O_DIRECTORY | O_RDONLY);
915+
if (procfd < 0) {
916+
SYSERROR("Unable to open /proc");
917+
shutdown(ipc_sockets[1], SHUT_RDWR);
918+
rexit(-1);
919+
}
920+
836921
/* attach now, create another subprocess later, since pid namespaces
837922
* only really affect the children of the current process
838923
*/
@@ -860,7 +945,8 @@ int lxc_attach(const char* name, const char* lxcpath, lxc_attach_exec_t exec_fun
860945
.options = options,
861946
.init_ctx = init_ctx,
862947
.exec_function = exec_function,
863-
.exec_payload = exec_payload
948+
.exec_payload = exec_payload,
949+
.procfd = procfd
864950
};
865951
/* We use clone_parent here to make this subprocess a direct child of
866952
* the initial process. Then this intermediate process can exit and
@@ -898,6 +984,7 @@ static int attach_child_main(void* data)
898984
{
899985
struct attach_clone_payload* payload = (struct attach_clone_payload*)data;
900986
int ipc_socket = payload->ipc_socket;
987+
int procfd = payload->procfd;
901988
lxc_attach_options_t* options = payload->options;
902989
struct lxc_proc_context_info* init_ctx = payload->init_ctx;
903990
#if HAVE_SYS_PERSONALITY_H
@@ -1038,21 +1125,11 @@ static int attach_child_main(void* data)
10381125
close(ipc_socket);
10391126

10401127
/* set new apparmor profile/selinux context */
1041-
if ((options->namespaces & CLONE_NEWNS) && (options->attach_flags & LXC_ATTACH_LSM)) {
1128+
if ((options->namespaces & CLONE_NEWNS) && (options->attach_flags & LXC_ATTACH_LSM) && init_ctx->lsm_label) {
10421129
int on_exec;
1043-
int proc_mounted;
10441130

10451131
on_exec = options->attach_flags & LXC_ATTACH_LSM_EXEC ? 1 : 0;
1046-
proc_mounted = mount_proc_if_needed("/");
1047-
if (proc_mounted == -1) {
1048-
ERROR("Error mounting a sane /proc");
1049-
rexit(-1);
1050-
}
1051-
ret = lsm_process_label_set(init_ctx->lsm_label,
1052-
init_ctx->container->lxc_conf, 0, on_exec);
1053-
if (proc_mounted)
1054-
umount("/proc");
1055-
if (ret < 0) {
1132+
if (lsm_set_label_at(procfd, on_exec, init_ctx->lsm_label) < 0) {
10561133
rexit(-1);
10571134
}
10581135
}
@@ -1103,6 +1180,9 @@ static int attach_child_main(void* data)
11031180
}
11041181
}
11051182

1183+
/* we don't need proc anymore */
1184+
close(procfd);
1185+
11061186
/* we're done, so we can now do whatever the user intended us to do */
11071187
rexit(payload->exec_function(payload->exec_payload));
11081188
}

0 commit comments

Comments
 (0)