Skip to content

Commit bd8ad4b

Browse files
committed
fix: improve socket security and refactor hook execution
1. Move package manager socket to dedicated directory /tmp/linglong/ 2. Set socket file permissions to 0700 (owner read/write only) 3. Clean up stale socket files from previous crashes on startup 4. Improve error logging to include socket path in failure messages 5. Refactor hook command execution to use Cmd class instead of std::system() 6. Remove custom shell escaping logic in favor of Cmd::exec() method Influence: 1. Test package manager daemon startup in no-dbus mode 2. Verify socket directory creation with correct permissions 3. Verify socket file has 0600 permissions after creation 4. Test daemon restart after crash to verify stale socket cleanup 5. Test package installation with pre/post install hooks 6. Verify hook commands execute correctly with environment variables 7. Test hook execution failure handling and error reporting 8. Verify socket cleanup on normal daemon shutdown
1 parent 6525cb7 commit bd8ad4b

3 files changed

Lines changed: 81 additions & 55 deletions

File tree

apps/ll-package-manager/src/main.cpp

Lines changed: 68 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,24 @@
1414
#include "linglong/repo/ostree_repo.h"
1515
#include "linglong/utils/error/error.h"
1616
#include "linglong/utils/file.h"
17+
#include "linglong/utils/log/log.h"
1718
#include "ocppi/cli/CLI.hpp"
1819
#include "ocppi/cli/crun/Crun.hpp"
1920

2021
#include <CLI/CLI.hpp>
2122

2223
#include <QCoreApplication>
2324

25+
#include <cerrno>
26+
#include <cstring>
2427
#include <filesystem>
2528
#include <memory>
2629
#include <string>
2730

31+
#include <sys/stat.h>
32+
#include <sys/types.h>
33+
#include <unistd.h>
34+
2835
namespace {
2936

3037
struct CommandLineOptions
@@ -162,18 +169,71 @@ auto runDaemonMode(ocppi::cli::CLI &cli, const CommandLineOptions &options) -> i
162169
} else {
163170
LogI("Running linglong package manager without dbus daemon...");
164171

165-
auto *server = new QDBusServer("unix:path=/tmp/linglong-package-manager.socket",
172+
// Create socket directory with secure permissions (0700)
173+
auto socketDir = std::filesystem::path("/tmp/linglong");
174+
175+
// Use mkdir to atomically create directory with correct permissions
176+
// This avoids TOCTOU race condition between check and create
177+
if (mkdir(socketDir.c_str(), S_IRWXU) == 0) {
178+
} else if (errno == EEXIST) {
179+
struct stat st;
180+
if (lstat(socketDir.c_str(), &st) != 0) {
181+
LogE("Failed to stat /tmp/linglong: {}", strerror(errno));
182+
return -1;
183+
}
184+
if (S_ISLNK(st.st_mode)) {
185+
LogE("Failed to create /tmp/linglong: it is a symbolic link. Please remove it.");
186+
return -1;
187+
}
188+
if (!S_ISDIR(st.st_mode)) {
189+
LogE("Failed to create /tmp/linglong: it exists but is not a directory. Please "
190+
"remove it.");
191+
return -1;
192+
}
193+
if (st.st_uid != getuid()) {
194+
LogE("Failed to create /tmp/linglong: owner mismatch (expected uid {}, got {}). "
195+
"Please remove it.",
196+
getuid(),
197+
st.st_uid);
198+
return -1;
199+
}
200+
if (chmod(socketDir.c_str(), S_IRWXU) != 0) {
201+
LogE("Failed to set permissions on /tmp/linglong: {}", strerror(errno));
202+
return -1;
203+
}
204+
} else {
205+
LogE("Failed to create /tmp/linglong: {}", strerror(errno));
206+
return -1;
207+
}
208+
209+
auto socketPath = socketDir / "linglong-package-manager.socket";
210+
211+
// Clean up stale socket from previous crash
212+
std::error_code ec;
213+
std::filesystem::remove(socketPath, ec);
214+
215+
auto *server = new QDBusServer(QString("unix:path=%1").arg(socketPath.c_str()),
166216
QCoreApplication::instance());
167217
if (!server->isConnected()) {
168-
LogE("listen on socket: {}", server->lastError().message().toStdString());
218+
LogE("listen on socket {}: {}",
219+
socketPath.c_str(),
220+
server->lastError().message().toStdString());
169221
return -1;
170222
}
171-
QObject::connect(QCoreApplication::instance(), &QCoreApplication::aboutToQuit, []() {
172-
if (QDir::root().remove("/tmp/linglong-package-manager.socket")) {
173-
return;
174-
}
175-
LogE("failed to remove /tmp/linglong-package-manager.socket.");
176-
});
223+
224+
// Set socket file permissions: only owner can read/write (0600)
225+
if (chmod(socketPath.c_str(), S_IRUSR | S_IWUSR) != 0) {
226+
LogE("Failed to set socket permissions: {}", strerror(errno));
227+
return -1;
228+
}
229+
230+
// Clean up on exit
231+
QObject::connect(QCoreApplication::instance(),
232+
&QCoreApplication::aboutToQuit,
233+
[socketPath]() {
234+
std::error_code ec;
235+
std::filesystem::remove(socketPath, ec);
236+
});
177237

178238
QObject::connect(server,
179239
&QDBusServer::newConnection,

libs/linglong/src/linglong/cli/cli.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,8 @@ utils::error::Result<api::dbus::v1::PackageManager *> Cli::getPkgMan()
470470
}
471471

472472
LogW("some subcommands will failed in --no-dbus mode.");
473-
const auto pkgManAddress = QString("unix:path=/tmp/linglong-package-manager.socket");
473+
const auto pkgManAddress =
474+
QString("unix:path=/tmp/linglong/linglong-package-manager.socket");
474475
QProcess::startDetached("sudo",
475476
{ "--user",
476477
LINGLONG_USERNAME,

libs/utils/src/linglong/utils/hooks.cpp

Lines changed: 11 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,21 @@
11
/*
2-
* SPDX-FileCopyrightText: 2025 UnionTech Software Technology Co., Ltd.
2+
* SPDX-FileCopyrightText: 2025 - 2026 UnionTech Software Technology Co., Ltd.
33
*
44
* SPDX-License-Identifier: LGPL-3.0-or-later
55
*/
66

77
#include "hooks.h"
88

9+
#include "cmd.h"
910
#include "configure.h"
1011
#include "linglong/common/error.h"
11-
#include "linglong/utils/env.h"
1212
#include "linglong/utils/error/error.h"
1313
#include "linglong/utils/log/log.h"
1414

1515
#include <fmt/format.h>
1616

1717
#include <array>
1818
#include <climits>
19-
#include <cstdlib>
2019
#include <cstring>
2120
#include <filesystem>
2221
#include <fstream>
@@ -33,58 +32,24 @@ constexpr std::string_view PRE_INSTALL_ACTION_PREFIX = "ll-pre-install=";
3332
constexpr std::string_view POST_INSTALL_ACTION_PREFIX = "ll-post-install=";
3433
constexpr std::string_view POST_UNINSTALL_ACTION_PREFIX = "ll-post-uninstall=";
3534

36-
// This function ensures the command string is safely wrapped for 'sh -c'.
37-
static std::string escapeAndWrapCommandForShell(const std::string &command)
38-
{
39-
std::string escapedCommand = command;
40-
size_t pos = 0;
41-
// Replace ' with '\'' (close current quote, add literal single quote, open new quote)
42-
while ((pos = escapedCommand.find('\'', pos)) != std::string::npos) {
43-
escapedCommand.replace(pos, 1, "'\\''");
44-
pos += 4;
45-
}
46-
47-
return "sh -c " + escapedCommand;
48-
}
49-
5035
using CommandList = const std::vector<std::string> &;
5136

5237
utils::error::Result<void> executeHookCommands(
5338
CommandList commands, const std::vector<std::pair<std::string, std::string>> &envVars) noexcept
5439
{
55-
LINGLONG_TRACE("Executing command");
40+
LINGLONG_TRACE("Executing hook commands");
5641

57-
std::vector<std::unique_ptr<EnvironmentVariableGuard>> envVarGuards;
58-
envVarGuards.reserve(envVars.size());
59-
for (const auto &pair : envVars) {
60-
envVarGuards.emplace_back(
61-
std::make_unique<EnvironmentVariableGuard>(pair.first, pair.second));
62-
}
42+
for (const auto &command : commands) {
43+
Cmd cmd("/bin/sh");
6344

64-
for (const auto &command_raw : commands) {
65-
std::string fullCommand = escapeAndWrapCommandForShell(command_raw);
66-
67-
int ret = std::system(fullCommand.c_str());
68-
69-
if (ret == -1) {
70-
return LINGLONG_ERR(fmt::format("Failed to execute command: '{}'. System error: {}.",
71-
fullCommand,
72-
common::error::errorString(errno)));
45+
for (const auto &[name, value] : envVars) {
46+
cmd.setEnv(name, value);
7347
}
7448

75-
if (!WIFEXITED(ret)) {
76-
int signalNum = WTERMSIG(ret);
77-
return LINGLONG_ERR(fmt::format("Command '{}' terminated by signal {} ({}).",
78-
fullCommand,
79-
signalNum,
80-
strsignal(signalNum)));
81-
}
82-
83-
int exitStatus = WEXITSTATUS(ret);
84-
if (exitStatus != 0) {
85-
return LINGLONG_ERR(fmt::format("Command '{}' exited with non-zero status: {}.",
86-
fullCommand,
87-
exitStatus));
49+
auto result = cmd.exec({ "-c", command });
50+
if (!result.has_value()) {
51+
return LINGLONG_ERR(
52+
fmt::format("Hook command '{}' failed: {}.", command, result.error()));
8853
}
8954
}
9055
return LINGLONG_OK;

0 commit comments

Comments
 (0)