mirror of
https://github.com/flatpak/flatpak.git
synced 2026-09-18 02:38:10 -04:00
revokefs: Avoid symlink path traversal in chmod()
Use glnx_chaseat to ensure that all the attacker-controlled paths end up inside the basefd directory, and then AT_SYMLINK_NOFOLLOW to make sure that the last path component won't escape from the basefd. On kernels >= 6.6 we can now use fchmodat() with AT_SYMLINK_NOFOLLOW, but on older kernels that didn't work, so if necessary fall back to opening the file with O_NOFOLLOW and then calling fchmod() on it. Because this is the last syscall that used the previous (flawed) validation mechanism, we can now remove the validation helpers and be sure that everything is using the glnx-chaseat()-based replacements. [smcv: Separated from a larger commit for better reviewability] Co-authored-by: Simon McVittie <smcv@collabora.com> Resolves: https://github.com/flatpak/flatpak/security/advisories/GHSA-qrwq-7qwx-q9rp
This commit is contained in:
1 parent
c68be6274e
commit
aa19390539
1 file changed
+21
-48
+21
-48
@@ -290,46 +290,6 @@ chase_request_any_and_path (RevokefsRequest *request,
|
||||
data_size - request->arg1, out_name);
|
||||
}
|
||||
|
||||
static gboolean
|
||||
validate_path (char *path)
|
||||
{
|
||||
char *end_segment;
|
||||
|
||||
/* No absolute or empty paths */
|
||||
if (*path == '/' || *path == 0)
|
||||
return FALSE;
|
||||
|
||||
while (*path != 0)
|
||||
{
|
||||
end_segment = strchr (path, '/');
|
||||
if (end_segment == NULL)
|
||||
end_segment = path + strlen (path);
|
||||
|
||||
if (strncmp (path, "..", 2) == 0)
|
||||
return FALSE;
|
||||
|
||||
path = end_segment;
|
||||
while (*path == '/')
|
||||
path++;
|
||||
}
|
||||
|
||||
return TRUE;
|
||||
}
|
||||
|
||||
static char *
|
||||
get_valid_path (guchar *data, size_t len)
|
||||
{
|
||||
char *path = g_strndup ((const char *) data, len);
|
||||
|
||||
if (!validate_path (path))
|
||||
{
|
||||
g_printerr ("Invalid path argument %s\n", path);
|
||||
exit (1);
|
||||
}
|
||||
|
||||
return path;
|
||||
}
|
||||
|
||||
static int
|
||||
mask_mode (int mode)
|
||||
{
|
||||
@@ -496,17 +456,30 @@ handle_chmod (RevokefsRequest *request,
|
||||
gsize data_size,
|
||||
RevokefsResponse *response)
|
||||
{
|
||||
g_autofree char *path = get_valid_path (request->data, data_size);
|
||||
g_autofree char *name = NULL;
|
||||
glnx_autofd int parent_fd = chase_request_path (request, data_size, &name);
|
||||
glnx_autofd int fd = -1;
|
||||
int mode = request->arg1;
|
||||
|
||||
/* Note we can't use AT_SYMLINK_NOFOLLOW yet;
|
||||
* https://marc.info/?l=linux-kernel&m=148830147803162&w=2
|
||||
* https://marc.info/?l=linux-fsdevel&m=149193779929561&w=2
|
||||
*/
|
||||
if (fchmodat (basefd, path, mask_mode (mode), 0) != 0)
|
||||
response->result = -errno;
|
||||
if (fchmodat (parent_fd, name, mask_mode (mode), AT_SYMLINK_NOFOLLOW) == 0)
|
||||
{
|
||||
response->result = 0;
|
||||
}
|
||||
else if (errno == ENOTSUP)
|
||||
{
|
||||
/* Linux < 6.6 didn't implement fchmodat() with AT_SYMLINK_NOFOLLOW */
|
||||
fd = openat (parent_fd, name, O_RDONLY | O_NOFOLLOW | O_NONBLOCK | O_CLOEXEC);
|
||||
if (fd == -1)
|
||||
response->result = -errno;
|
||||
else if (fchmod (fd, mask_mode (mode)) != 0)
|
||||
response->result = -errno;
|
||||
else
|
||||
response->result = 0;
|
||||
}
|
||||
else
|
||||
response->result = 0;
|
||||
{
|
||||
response->result = -errno;
|
||||
}
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user