Check for symlink target length errors

Check for symlink length errors at the VFS library and the ram_fs and
vfs servers.

Fix #2462
This commit is contained in:
Emery Hemingway 2017-07-12 10:05:38 -05:00 committed by Christian Helmuth
parent cfdac3f4c3
commit 2deddf1e6d
5 changed files with 46 additions and 12 deletions

View File

@ -706,7 +706,7 @@ int Libc::Vfs_plugin::symlink(const char *oldpath, const char *newpath)
case Result::SYMLINK_ERR_EXISTS: errno = EEXIST; return -1; case Result::SYMLINK_ERR_EXISTS: errno = EEXIST; return -1;
case Result::SYMLINK_ERR_NO_ENTRY: errno = ENOENT; return -1; case Result::SYMLINK_ERR_NO_ENTRY: errno = ENOENT; return -1;
case Result::SYMLINK_ERR_NAME_TOO_LONG: errno = ENAMETOOLONG; return -1; case Result::SYMLINK_ERR_NAME_TOO_LONG: errno = ENAMETOOLONG; return -1;
case Result::SYMLINK_ERR_NO_PERM: errno = ENOSYS; return -1; case Result::SYMLINK_ERR_NO_PERM: errno = EPERM; return -1;
case Result::SYMLINK_ERR_NO_SPACE: errno = ENOSPC; return -1; case Result::SYMLINK_ERR_NO_SPACE: errno = ENOSPC; return -1;
case Result::SYMLINK_OK: break; case Result::SYMLINK_OK: break;
} }

View File

@ -530,6 +530,8 @@ class Vfs::Fs_file_system : public File_system
Symlink_result symlink(char const *from, char const *to) override Symlink_result symlink(char const *from, char const *to) override
{ {
auto const from_len = strlen(from);
/* /*
* We write to the symlink via the packet stream. Hence we need * We write to the symlink via the packet stream. Hence we need
* to serialize with other packet-stream operations. * to serialize with other packet-stream operations.
@ -550,10 +552,18 @@ class Vfs::Fs_file_system : public File_system
Fs_handle_guard from_dir_guard(*this, _fs, dir_handle, _handle_space); Fs_handle_guard from_dir_guard(*this, _fs, dir_handle, _handle_space);
::File_system::Symlink_handle symlink_handle = ::File_system::Symlink_handle symlink_handle =
_fs.symlink(dir_handle, symlink_name.base() + 1, true); _fs.symlink(dir_handle, symlink_name.base() + 1, true);
Fs_handle_guard symlink_guard(*this, _fs, symlink_handle, _handle_space); Fs_handle_guard symlink_guard(*this, _fs, symlink_handle, _handle_space);
_write(symlink_guard, from, strlen(from) + 1, 0); auto const n = _write(symlink_guard, from, from_len, 0);
/*
* a convention at the VFS server is to return an invalid
* result length when the target is too long
*/
if (n != from_len) {
return n ? SYMLINK_ERR_NAME_TOO_LONG : SYMLINK_ERR_NO_PERM;
}
} }
catch (::File_system::Invalid_handle) { return SYMLINK_ERR_NO_ENTRY; } catch (::File_system::Invalid_handle) { return SYMLINK_ERR_NO_ENTRY; }
catch (::File_system::Node_already_exists) { return SYMLINK_ERR_EXISTS; } catch (::File_system::Node_already_exists) { return SYMLINK_ERR_EXISTS; }

View File

@ -246,7 +246,7 @@ class Vfs_ram::Symlink : public Vfs_ram::Node
void set(char const *target, size_t len) void set(char const *target, size_t len)
{ {
_len = min(len, MAX_PATH_LEN); _len = len;
memcpy(_target, target, _len); memcpy(_target, target, _len);
} }
@ -579,6 +579,10 @@ class Vfs::Ram_file_system : public Vfs::File_system
{ {
using namespace Vfs_ram; using namespace Vfs_ram;
auto const target_len = strlen(target);
if (target_len > MAX_PATH_LEN)
return SYMLINK_ERR_NAME_TOO_LONG;
Symlink *link; Symlink *link;
Directory *parent = lookup_parent(path); Directory *parent = lookup_parent(path);
if (!parent) return SYMLINK_ERR_NO_ENTRY; if (!parent) return SYMLINK_ERR_NO_ENTRY;
@ -606,7 +610,7 @@ class Vfs::Ram_file_system : public Vfs::File_system
} }
if (*target) if (*target)
link->set(target, strlen(target)); link->set(target, target_len);
link->unlock(); link->unlock();
return SYMLINK_OK; return SYMLINK_OK;
} }

View File

@ -43,8 +43,6 @@ class Ram_fs::Symlink : public Node
/* Ideal symlink operations are atomic. */ /* Ideal symlink operations are atomic. */
if (seek_offset) return 0; if (seek_offset) return 0;
len = min(len, sizeof(_link_to));
for (size_t i = 0; i < len; ++i) { for (size_t i = 0; i < len; ++i) {
if (src[i] == '\0') { if (src[i] == '\0') {
len = i; len = i;
@ -52,6 +50,13 @@ class Ram_fs::Symlink : public Node
} }
} }
/*
* if the target is too long return a
* short result to indicate the error
*/
if (len > sizeof(_link_to))
return len >> 1;
Genode::memcpy(_link_to, src, len); Genode::memcpy(_link_to, src, len);
_len = len; _len = len;
return len; return len;

View File

@ -132,15 +132,30 @@ struct Vfs_server::Symlink : Node
size_t write(Vfs::File_system &vfs, char const *src, size_t len, seek_off_t seek_offset) size_t write(Vfs::File_system &vfs, char const *src, size_t len, seek_off_t seek_offset)
{ {
/* ensure symlink gets something null-terminated */ /*
Genode::String<MAX_PATH_LEN> target(Genode::Cstring(src, len)); * if the symlink target is too long return a short result
* because a competent File_system client will error on a
* length mismatch
*/
if (vfs.symlink(target.string(), path()) == Directory_service::SYMLINK_OK) if (len > MAX_PATH_LEN) {
return 0; return len >> 1;
}
/* ensure symlink gets something null-terminated */
Genode::String<MAX_PATH_LEN+1> target(Genode::Cstring(src, len));
size_t const target_len = target.length()-1;
switch (vfs.symlink(target.string(), path())) {
case Directory_service::SYMLINK_OK: break;
case Directory_service::SYMLINK_ERR_NAME_TOO_LONG:
return target_len >> 1;
default: return 0;
}
mark_as_updated(); mark_as_updated();
notify_listeners(); notify_listeners();
return target.length(); return target_len;
} }
bool read_ready() override { return true; } bool read_ready() override { return true; }