mirror
/
transmission
mirror of https://github.com/transmission/transmission
1
0
Fork 0
Code Issues Releases Wiki Activity

fix: env var leak in tr_spawn_async() (#2212) 

* fix: env var leak in tr_spawn_async()
This commit is contained in:
Charles Kerr 2021-11-24 13:25:23 -06:00 committed by GitHub
parent d4e0f368c8
commit dbd9130d9d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 90 additions and 111 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
libtransmission/subprocess-posix.cc
View File

@ -52,16 +52,23 @@ static void set_system_error(tr_error** error, int code, char const* what)
}
}
static bool tr_spawn_async_in_child(char* const* cmd, char* const* env, char const* work_dir, int pipe_fd)
static bool tr_spawn_async_in_child(
char const* const* cmd,
std::map<std::string_view, std::string_view> const& env,
char const* work_dir,
int pipe_fd)
{
if (env != nullptr)
auto key_sz = std::string{};
auto val_sz = std::string{};
for (auto const& [key_sv, val_sv] : env)
{
for (size_t i = 0; env[i] != nullptr; ++i)
key_sz = key_sv;
val_sz = val_sv;
if (setenv(key_sz.c_str(), val_sz.c_str(), 1) != 0)
{
if (putenv(env[i]) != 0)
{
goto FAIL;
}
goto FAIL;
}
}
@ -70,7 +77,7 @@ static bool tr_spawn_async_in_child(char* const* cmd, char* const* env, char con
goto FAIL;
}
if (execvp(cmd[0], cmd) == -1)
if (execvp(cmd[0], const_cast<char* const*>(cmd)) == -1)
{
goto FAIL;
}
@ -115,7 +122,11 @@ static bool tr_spawn_async_in_parent(int pipe_fd, tr_error** error)
return true;
}
bool tr_spawn_async(char* const* cmd, char* const* env, char const* work_dir, tr_error** error)
bool tr_spawn_async(
char const* const* cmd,
std::map<std::string_view, std::string_view> const& env,
char const* work_dir,
tr_error** error)
{
static bool sigchld_handler_set = false;

39
libtransmission/subprocess-win32.cc
View File

@ -20,6 +20,8 @@
#include "tr-assert.h"
#include "utils.h"
using namespace std::literals;
enum tr_app_type
{
TR_APP_TYPE_EXE,
@ -128,28 +130,19 @@ static int compare_env_part_names(void const* vlhs, void const* vrhs)
return ret;
}
static wchar_t** to_wide_env(char const* const* env)
static wchar_t** to_wide_env(std::map<std::string_view, std::string_view> const& env)
{
if (env == nullptr || env[0] == nullptr)
auto const part_count = std::size(env);
wchar_t** const wide_env = tr_new(wchar_t*, part_count + 1);
int i = 0;
for (auto const& [key_sv, val_sv] : env)
{
return nullptr;
auto const line = tr_strvJoin(key_sv, "="sv, val_sv);
wide_env[i++] = tr_win32_utf8_to_native(std::data(line), std::size(line));
}
size_t part_count = 0;
while (env[part_count] != nullptr)
{
++part_count;
}
wchar_t** wide_env = tr_new(wchar_t*, part_count + 1);
for (size_t i = 0; i < part_count; ++i)
{
wide_env[i] = tr_win32_utf8_to_native(env[i], -1);
}
wide_env[part_count] = nullptr;
wide_env[i] = nullptr;
TR_ASSERT(i == part_count);
/* "The sort is case-insensitive, Unicode order, without regard to locale" (c) MSDN */
qsort(wide_env, part_count, sizeof(wchar_t*), &compare_env_part_names);
@ -157,7 +150,7 @@ static wchar_t** to_wide_env(char const* const* env)
return wide_env;
}
static bool create_env_block(char const* const* env, wchar_t** env_block, tr_error** error)
static bool create_env_block(std::map<std::string_view, std::string_view> const& env, wchar_t** env_block, tr_error** error)
{
wchar_t** wide_env = to_wide_env(env);
@ -379,7 +372,11 @@ cleanup:
return ret;
}
bool tr_spawn_async(char* const* cmd, char* const* env, char const* work_dir, tr_error** error)
bool tr_spawn_async(
char const* const* cmd,
std::map<std::string_view, std::string_view> const& env,
char const* work_dir,
tr_error** error)
{
wchar_t* env_block = nullptr;

9
libtransmission/subprocess.h
View File

@ -8,6 +8,13 @@
#pragma once
#include <map>
#include <string_view>
#include "tr-macros.h"
bool tr_spawn_async(char* const* cmd, char* const* env, char const* work_dir, struct tr_error** error);
bool tr_spawn_async(
char const* const* cmd,
std::map<std::string_view, std::string_view> const& env,
char const* work_dir,
struct tr_error** error);

25
libtransmission/torrent.cc
View File

@ -1985,21 +1985,20 @@ static void torrentCallScript(tr_torrent const* tor, char const* script)
char* const torrent_dir = tr_sys_path_native_separators(tr_strdup(tor->currentDir));
char* const cmd[] = {
tr_strdup(script),
char const* const cmd[] = {
script,
nullptr,
};
char* const env[] = {
tr_strdup_printf("TR_APP_VERSION=%s", SHORT_VERSION_STRING),
tr_strdup_printf("TR_TIME_LOCALTIME=%s", ctime_str),
tr_strdup_printf("TR_TORRENT_DIR=%s", torrent_dir),
tr_strdup_printf("TR_TORRENT_HASH=%s", tor->info.hashString),
tr_strdup_printf("TR_TORRENT_ID=%d", tr_torrentId(tor)),
tr_strdup_printf("TR_TORRENT_LABELS=%s", buildLabelsString(tor).c_str()),
tr_strdup_printf("TR_TORRENT_NAME=%s", tr_torrentName(tor)),
tr_strdup_printf("TR_TORRENT_TRACKERS=%s", buildTrackersString(tor).c_str()),
nullptr,
auto const env = std::map<std::string_view, std::string_view>{
{ "TR_APP_VERSION"sv, SHORT_VERSION_STRING },
{ "TR_TIME_LOCALTIME"sv, ctime_str },
{ "TR_TORRENT_DIR"sv, torrent_dir },
{ "TR_TORRENT_HASH"sv, tor->info.hashString },
{ "TR_TORRENT_ID"sv, std::to_string(tr_torrentId(tor)) },
{ "TR_TORRENT_LABELS"sv, buildLabelsString(tor) },
{ "TR_TORRENT_NAME"sv, tr_torrentName(tor) },
{ "TR_TORRENT_TRACKERS"sv, buildTrackersString(tor) },
};
tr_logAddTorInfo(tor, "Calling script \"%s\"", script);
@ -2012,8 +2011,6 @@ static void torrentCallScript(tr_torrent const* tor, char const* script)
tr_error_free(error);
}
tr_free_ptrv((void* const*)env);
tr_free_ptrv((void* const*)cmd);
tr_free(torrent_dir);
}

99
tests/libtransmission/subprocess-test.cc
View File

@ -87,14 +87,10 @@ TEST_P(SubprocessTest, SpawnAsyncMissingExec)
{
auto const missing_exe_path = std::string{ TR_IF_WIN32("C:\\", "/") "tr-missing-test-exe" TR_IF_WIN32(".exe", "") };
auto args = std::array<char*, 2>{
// FIXME(ckerr): remove tr_strdup()s after https://github.com/transmission/transmission/issues/1384
tr_strdup(missing_exe_path.data()),
nullptr
};
auto args = std::array<char const*, 2>{ missing_exe_path.data(), nullptr };
tr_error* error = nullptr;
auto const ret = tr_spawn_async(args.data(), nullptr, nullptr, &error);
auto const ret = tr_spawn_async(std::data(args), {}, nullptr, &error);
EXPECT_FALSE(ret);
EXPECT_NE(nullptr, error);
EXPECT_NE(0, error->code);
@ -113,20 +109,17 @@ TEST_P(SubprocessTest, SpawnAsyncArgs)
auto const test_arg3 = std::string{};
auto const test_arg4 = std::string{ "\"arg3'^! $PATH %PATH% \\" };
auto args = std::array<char*, 8>{
// FIXME(ckerr): remove tr_strdup()s after https://github.com/transmission/transmission/issues/1384
tr_strdup(self_path_.c_str()),
tr_strdup(result_path.data()),
tr_strdup(arg_dump_args_.data()),
tr_strdup(test_arg1.data()),
tr_strdup(test_arg2.data()),
tr_strdup(test_arg3.data()),
tr_strdup(allow_batch_metachars ? test_arg4.data() : nullptr),
nullptr
};
auto const args = std::array<char const*, 8>{ self_path_.c_str(),
result_path.data(),
arg_dump_args_.data(),
test_arg1.data(),
test_arg2.data(),
test_arg3.data(),
allow_batch_metachars ? test_arg4.data() : nullptr,
nullptr };
tr_error* error = nullptr;
bool const ret = tr_spawn_async(args.data(), nullptr, nullptr, &error);
bool const ret = tr_spawn_async(std::data(args), {}, nullptr, &error);
EXPECT_TRUE(ret) << args[0] << ' ' << args[1];
EXPECT_EQ(nullptr, error) << error->code << ", " << error->message;
@ -183,34 +176,31 @@ TEST_P(SubprocessTest, SpawnAsyncEnv)
auto const test_env_value4 = std::string{ "bar" };
auto const test_env_value5 = std::string{ "jar" };
auto args = std::array<char*, 10>{
// FIXME(ckerr): remove tr_strdup()s after https://github.com/transmission/transmission/issues/1384
tr_strdup(self_path_.c_str()), //
tr_strdup(result_path.data()), //
tr_strdup(arg_dump_env_.data()), //
tr_strdup(test_env_key1.data()), //
tr_strdup(test_env_key2.data()), //
tr_strdup(test_env_key3.data()), //
tr_strdup(test_env_key4.data()), //
tr_strdup(test_env_key5.data()), //
tr_strdup(test_env_key6.data()), //
auto args = std::array<char const*, 10>{
self_path_.c_str(), //
result_path.data(), //
arg_dump_env_.data(), //
test_env_key1.data(), //
test_env_key2.data(), //
test_env_key3.data(), //
test_env_key4.data(), //
test_env_key5.data(), //
test_env_key6.data(), //
nullptr, //
};
auto env = std::array<char*, 5>{
// FIXME(ckerr): remove tr_strdup()s after https://github.com/transmission/transmission/issues/1384
tr_strdup_printf("%s=%s", test_env_key1.data(), test_env_value1.data()),
tr_strdup_printf("%s=%s", test_env_key2.data(), test_env_value2.data()),
tr_strdup_printf("%s=%s", test_env_key3.data(), test_env_value3.data()),
tr_strdup_printf("%s=%s", test_env_key5.data(), test_env_value5.data()),
nullptr,
auto const env = std::map<std::string_view, std::string_view>{
{ test_env_key1, test_env_value1 },
{ test_env_key2, test_env_value2 },
{ test_env_key3, test_env_value3 },
{ test_env_key5, test_env_value5 },
};
setenv("FOO", "bar", true); // inherited
setenv("ZOO", "tar", true); // overridden
tr_error* error = nullptr;
bool const ret = tr_spawn_async(args.data(), env.data(), nullptr, &error);
bool const ret = tr_spawn_async(std::data(args), env, nullptr, &error);
EXPECT_TRUE(ret);
EXPECT_EQ(nullptr, error);
@ -248,11 +238,6 @@ TEST_P(SubprocessTest, SpawnAsyncEnv)
EXPECT_FALSE(tr_sys_file_read_line(fd, buffer.data(), buffer.size(), nullptr));
tr_sys_file_close(fd, nullptr);
for (auto& env_item : env)
{
tr_free(env_item);
}
}
TEST_P(SubprocessTest, SpawnAsyncCwdExplicit)
@ -260,16 +245,10 @@ TEST_P(SubprocessTest, SpawnAsyncCwdExplicit)
auto const test_dir = sandbox_.path();
auto const result_path = buildSandboxPath("result.txt");
auto args = std::array<char*, 4>{
// FIXME(ckerr): remove tr_strdup()s after https://github.com/transmission/transmission/issues/1384
tr_strdup(self_path_.c_str()),
tr_strdup(result_path.data()),
tr_strdup(arg_dump_cwd_.data()),
nullptr
};
auto const args = std::array<char const*, 4>{ self_path_.c_str(), result_path.data(), arg_dump_cwd_.data(), nullptr };
tr_error* error = nullptr;
bool const ret = tr_spawn_async(args.data(), nullptr, test_dir.c_str(), &error);
bool const ret = tr_spawn_async(std::data(args), {}, test_dir.c_str(), &error);
EXPECT_TRUE(ret);
EXPECT_EQ(nullptr, error);
@ -294,16 +273,10 @@ TEST_P(SubprocessTest, SpawnAsyncCwdInherit)
auto const result_path = buildSandboxPath("result.txt");
auto const expected_cwd = nativeCwd();
auto args = std::array<char*, 4>{
// FIXME(ckerr): remove tr_strdup()s after https://github.com/transmission/transmission/issues/1384
tr_strdup(self_path_.c_str()),
tr_strdup(result_path.data()),
tr_strdup(arg_dump_cwd_.data()),
nullptr
};
auto const args = std::array<char const*, 4>{ self_path_.c_str(), result_path.data(), arg_dump_cwd_.data(), nullptr };
tr_error* error = nullptr;
auto const ret = tr_spawn_async(args.data(), nullptr, nullptr, &error);
auto const ret = tr_spawn_async(std::data(args), {}, nullptr, &error);
EXPECT_TRUE(ret);
EXPECT_EQ(nullptr, error);
@ -323,16 +296,10 @@ TEST_P(SubprocessTest, SpawnAsyncCwdMissing)
{
auto const result_path = buildSandboxPath("result.txt");
auto args = std::array<char*, 4>{
// FIXME(ckerr): remove tr_strdup()s after https://github.com/transmission/transmission/issues/1384
tr_strdup(self_path_.c_str()),
tr_strdup(result_path.data()),
tr_strdup(arg_dump_cwd_.data()),
nullptr
};
auto const args = std::array<char const*, 4>{ self_path_.c_str(), result_path.data(), arg_dump_cwd_.data(), nullptr };
tr_error* error = nullptr;
auto const ret = tr_spawn_async(args.data(), nullptr, TR_IF_WIN32("C:\\", "/") "tr-missing-test-work-dir", &error);
auto const ret = tr_spawn_async(std::data(args), {}, TR_IF_WIN32("C:\\", "/") "tr-missing-test-work-dir", &error);
EXPECT_FALSE(ret);
EXPECT_NE(nullptr, error);
EXPECT_NE(0, error->code);