Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Escape mount sources and targets #3828

Merged
merged 2 commits into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion include/multipass/utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,6 @@ Str&& trim(Str&& s, Filter&& filter);
template <typename Str>
Str&& trim(Str&& s);
std::string& trim_newline(std::string& s);
std::string escape_char(const std::string& s, char c);
std::string escape_for_shell(const std::string& s);
std::vector<std::string> split(const std::string& string, const std::string& delimiter);
std::string match_line_for(const std::string& output, const std::string& matcher);
Expand Down
2 changes: 1 addition & 1 deletion src/platform/platform_linux.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ void mp::platform::Platform::create_alias_script(const std::string& alias, const

std::string multipass_exec = mpu::in_multipass_snap()
? "exec /usr/bin/snap run multipass"
: fmt::format("\"{}\"", QCoreApplication::applicationFilePath());
: fmt::format("{:?}", QCoreApplication::applicationFilePath().toStdString());

std::string script = "#!/bin/sh\n\n" + multipass_exec + " " + alias + " -- \"${@}\"\n";

Expand Down
20 changes: 11 additions & 9 deletions src/sshfs_mount/sftp_server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ void check_sshfs_status(mp::SSHSession& session, mp::SSHProcess& sshfs_process)
auto create_sshfs_process(mp::SSHSession& session, const std::string& sshfs_exec_line, const std::string& source,
const std::string& target)
{
auto sshfs_process = session.exec(fmt::format("sudo {} :\"{}\" \"{}\"", sshfs_exec_line, source, target));
auto sshfs_process = session.exec(fmt::format("sudo {} :{:?} {:?}", sshfs_exec_line, source, target));

check_sshfs_status(session, sshfs_process);

Expand Down Expand Up @@ -235,12 +235,16 @@ int reverse_id_for(const mp::id_mappings& id_maps, const int id, const int defau
}
} // namespace

mp::SftpServer::SftpServer(SSHSession&& session, const std::string& source, const std::string& target,
const id_mappings& gid_mappings, const id_mappings& uid_mappings, int default_uid,
int default_gid, const std::string& sshfs_exec_line)
mp::SftpServer::SftpServer(SSHSession&& session,
const std::string& source,
const std::string& target,
const id_mappings& gid_mappings,
const id_mappings& uid_mappings,
int default_uid,
int default_gid,
const std::string& sshfs_exec_line)
: ssh_session{std::move(session)},
sshfs_process{create_sshfs_process(ssh_session, sshfs_exec_line, mp::utils::escape_char(source, '"'),
mp::utils::escape_char(target, '"'))},
sshfs_process{create_sshfs_process(ssh_session, sshfs_exec_line, source, target)},
sftp_server_session{make_sftp_session(ssh_session, sshfs_process->release_channel())},
source_path{source},
target_path{target},
Expand Down Expand Up @@ -440,9 +444,7 @@ void mp::SftpServer::run()
ssh_session.exec(fmt::format("sudo umount {}", mount_path));
}

sshfs_process =
create_sshfs_process(ssh_session, sshfs_exec_line, mp::utils::escape_char(source_path, '"'),
mp::utils::escape_char(target_path, '"'));
sshfs_process = create_sshfs_process(ssh_session, sshfs_exec_line, source_path, target_path);
sftp_server_session = make_sftp_session(ssh_session, sshfs_process->release_channel());

continue;
Expand Down
21 changes: 8 additions & 13 deletions src/utils/utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -195,11 +195,6 @@ std::string& mp::utils::trim_newline(std::string& s)
return s;
}

std::string mp::utils::escape_char(const std::string& in, char c)
{
return std::regex_replace(in, std::regex({c}), fmt::format("\\{}", c));
}

// Escape all characters which need to be escaped in the shell.
std::string mp::utils::escape_for_shell(const std::string& in)
{
Expand All @@ -216,19 +211,19 @@ std::string mp::utils::escape_for_shell(const std::string& in)

for (char c : in)
{
if (0xa == c) // newline
if ('\n' == c) // newline
{
*ret_insert++ = 0x22; // double quotes
*ret_insert++ = 0xa; // newline
*ret_insert++ = 0x22; // double quotes
*ret_insert++ = '"'; // double quotes
*ret_insert++ = '\n'; // newline
*ret_insert++ = '"'; // double quotes
}
else
{
// If the character is in one of these code ranges, then it must be escaped.
if (c < 0x25 || c > 0x7a || (c > 0x25 && c < 0x2b) || (c > 0x5a && c < 0x5f) || 0x2c == c || 0x3b == c ||
0x3c == c || 0x3e == c || 0x3f == c || 0x60 == c)
{
*ret_insert++ = 0x5c; // backslash
*ret_insert++ = '\\'; // backslash
}

*ret_insert++ = c;
Expand Down Expand Up @@ -526,7 +521,7 @@ std::pair<std::string, std::string> mp::utils::get_path_split(mp::SSHSession& se

std::string existing = MP_UTILS.run_in_ssh_session(
session,
fmt::format("sudo /bin/bash -c 'P=\"{}\"; while [ ! -d \"$P/\" ]; do P=\"${{P%/*}}\"; done; echo $P/'",
fmt::format("sudo /bin/bash -c 'P={:?}; while [ ! -d \"$P/\" ]; do P=\"${{P%/*}}\"; done; echo $P/'",
absolute));

return {existing,
Expand All @@ -537,7 +532,7 @@ std::pair<std::string, std::string> mp::utils::get_path_split(mp::SSHSession& se
void mp::utils::make_target_dir(mp::SSHSession& session, const std::string& root, const std::string& relative_target)
{
MP_UTILS.run_in_ssh_session(session,
fmt::format("sudo /bin/bash -c 'cd \"{}\" && mkdir -p \"{}\"'", root, relative_target));
fmt::format("sudo /bin/bash -c 'cd {:?} && mkdir -p {:?}'", root, relative_target));
}

// Set ownership of all directories on a path starting on a given root.
Expand All @@ -546,7 +541,7 @@ void mp::utils::set_owner_for(mp::SSHSession& session, const std::string& root,
int vm_user, int vm_group)
{
MP_UTILS.run_in_ssh_session(session,
fmt::format("sudo /bin/bash -c 'cd \"{}\" && chown -R {}:{} \"{}\"'",
fmt::format("sudo /bin/bash -c 'cd {:?} && chown -R {}:{} {:?}'",
root,
vm_user,
vm_group,
Expand Down
7 changes: 0 additions & 7 deletions tests/test_utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -455,13 +455,6 @@ TEST(Utils, trim_newline_assertion_works)
ASSERT_DEBUG_DEATH(mp::utils::trim_newline(s), "[Aa]ssert");
}

TEST(Utils, escape_char_actually_escapes)
{
std::string s{"I've got \"quotes\""};
auto res = mp::utils::escape_char(s, '"');
EXPECT_THAT(res, ::testing::StrEq("I've got \\\"quotes\\\""));
}

TEST(Utils, escape_for_shell_actually_escapes)
{
std::string s{"I've got \"quotes\""};
Expand Down
Loading