diff --git a/Buffer.cc b/Buffer.cc index a34a390..a5f94d7 100644 --- a/Buffer.cc +++ b/Buffer.cc @@ -175,6 +175,11 @@ atomic_write_file(const std::string &path_in, const char *data, std::size_t len, struct stat dst_st{}; const bool dst_exists = ::stat(path.c_str(), &dst_st) == 0; + // Never replace a directory, device node, FIFO or socket with a file. + if (dst_exists && !S_ISREG(dst_st.st_mode)) { + err = "Refusing to overwrite a non-regular file: " + path; + return false; + } // A file with other hard links must be rewritten in place, or those // names would keep the old content. @@ -484,7 +489,15 @@ Buffer::OpenFromFile(const std::string &path, std::string &err) const std::string norm = normalize_path(path); // If the file doesn't exist, initialize an empty, non-file-backed buffer // with the provided filename. Do not touch the filesystem until Save/SaveAs. - if (!std::filesystem::exists(norm)) { + std::error_code exists_ec; + const bool exists = std::filesystem::exists(norm, exists_ec); + if (exists_ec) { + // e.g. name too long, or a parent directory we may not search + err = "Cannot open " + norm + ": " + exists_ec.message(); + kte::ErrorHandler::Instance().Error("Buffer", err, norm); + return false; + } + if (!exists) { rows_.clear(); nrows_ = 0; filename_ = norm; @@ -508,50 +521,52 @@ Buffer::OpenFromFile(const std::string &path, std::string &err) return true; } - std::ifstream in(norm, std::ios::in | std::ios::binary); - if (!in) { - err = "Failed to open file: " + norm; + // Open non-blocking so a named pipe cannot hang the editor, then accept + // regular files only: a directory reads as garbage (or throws), a FIFO + // blocks, and saving over a device node would replace it with a file. + int oflags = O_RDONLY | O_NONBLOCK; +#ifdef O_CLOEXEC + oflags |= O_CLOEXEC; +#endif + const int fd = kte::syscall::Open(norm.c_str(), oflags); + if (fd < 0) { + err = "Failed to open file: " + norm + ": " + std::strerror(errno); + kte::ErrorHandler::Instance().Error("Buffer", err, norm); + return false; + } + struct stat st{}; + if (kte::syscall::Fstat(fd, &st) != 0 || !S_ISREG(st.st_mode)) { + err = S_ISDIR(st.st_mode) ? ("Is a directory: " + norm) : ("Not a regular file: " + norm); + (void) kte::syscall::Close(fd); kte::ErrorHandler::Instance().Error("Buffer", err, norm); return false; } - // Read entire file into PieceTable as-is + // Read to EOF in chunks (the file may change size while we read). std::string data; - in.seekg(0, std::ios::end); - if (!in) { - err = "Failed to seek to end of file: " + norm; + try { + data.reserve(static_cast(st.st_size)); + char chunk[1 << 16]; + for (;;) { + const ssize_t n = ::read(fd, chunk, sizeof(chunk)); + if (n > 0) { + data.append(chunk, static_cast(n)); + } else if (n == 0) { + break; + } else if (errno != EINTR) { + err = "Failed to read file: " + norm + ": " + std::strerror(errno); + (void) kte::syscall::Close(fd); + kte::ErrorHandler::Instance().Error("Buffer", err, norm); + return false; + } + } + } catch (const std::bad_alloc &) { + (void) kte::syscall::Close(fd); + err = "File too large to load: " + norm; kte::ErrorHandler::Instance().Error("Buffer", err, norm); return false; } - auto sz = in.tellg(); - if (sz < 0) { - err = "Failed to get file size: " + norm; - kte::ErrorHandler::Instance().Error("Buffer", err, norm); - return false; - } - if (sz > 0) { - data.resize(static_cast(sz)); - in.seekg(0, std::ios::beg); - if (!in) { - err = "Failed to seek to beginning of file: " + norm; - kte::ErrorHandler::Instance().Error("Buffer", err, norm); - return false; - } - in.read(data.data(), static_cast(data.size())); - if (!in && !in.eof()) { - err = "Failed to read file: " + norm; - kte::ErrorHandler::Instance().Error("Buffer", err, norm); - return false; - } - // Validate we read the expected number of bytes - const std::streamsize bytes_read = in.gcount(); - if (bytes_read != static_cast(data.size())) { - err = "Partial read of file (expected " + std::to_string(data.size()) + - " bytes, got " + std::to_string(bytes_read) + "): " + norm; - kte::ErrorHandler::Instance().Error("Buffer", err, norm); - return false; - } - } + (void) kte::syscall::Close(fd); content_.Clear(); if (!data.empty()) content_.Append(data.data(), data.size()); diff --git a/Command.cc b/Command.cc index 2c3b57b..1da39fe 100644 --- a/Command.cc +++ b/Command.cc @@ -11,6 +11,7 @@ #include #include "Command.h" +#include "ErrorHandler.h" #include "RegexGuard.h" #include "TermWidth.h" #include "syntax/HighlighterRegistry.h" @@ -342,6 +343,26 @@ is_mutating_command(CommandId id) } +// Non-throwing filesystem queries for command code. The throwing overloads +// fail on ENAMETOOLONG, EACCES or a deleted working directory, which used +// to terminate the editor. +static bool +fs_exists(const std::string &path) +{ + std::error_code ec; + return std::filesystem::exists(path, ec) && !ec; +} + + +static std::filesystem::path +safe_current_path() +{ + std::error_code ec; + auto p = std::filesystem::current_path(ec); + return ec ? std::filesystem::path(".") : p; +} + + // True if `path` names the file `buf` is backed by (same inode). static bool is_buffers_own_file(const Buffer &buf, const std::string &path) @@ -906,7 +927,7 @@ cmd_save(CommandContext &ctx) if (!buf->IsFileBacked()) { if (!buf->Filename().empty()) { // If first-time save to an existing path, confirm overwrite - if (std::filesystem::exists(buf->Filename())) { + if (fs_exists(buf->Filename())) { ctx.editor.StartPrompt(Editor::PromptKind::Confirm, "Overwrite", ""); ctx.editor.SetPendingOverwritePath(buf->Filename()); ctx.editor.SetStatus( @@ -997,7 +1018,7 @@ cmd_save_as(CommandContext &ctx) return false; } // Ask before replacing an existing file other than the buffer's own. - if (std::filesystem::exists(ctx.arg) && !is_buffers_own_file(*buf, ctx.arg)) { + if (fs_exists(ctx.arg) && !is_buffers_own_file(*buf, ctx.arg)) { ctx.editor.StartPrompt(Editor::PromptKind::Confirm, "Overwrite", ""); ctx.editor.SetPendingOverwritePath(ctx.arg); ctx.editor.SetStatus(std::string("Overwrite existing file '") + ctx.arg + "'? (y/N)"); @@ -1846,7 +1867,7 @@ cmd_visual_file_picker_toggle(const CommandContext &ctx) // Initialize directory to current working directory if empty if (ctx.editor.FilePickerDir().empty()) { try { - ctx.editor.SetFilePickerDir(std::filesystem::current_path().string()); + ctx.editor.SetFilePickerDir(safe_current_path().string()); } catch (...) { ctx.editor.SetFilePickerDir("."); } @@ -2074,17 +2095,18 @@ cmd_insert_text(CommandContext &ctx) std::filesystem::path p(expanded); std::filesystem::path dir; std::string base; + std::error_code dir_ec; if (expanded.empty()) { - dir = std::filesystem::current_path(); + dir = safe_current_path(); base.clear(); - } else if (std::filesystem::is_directory(p)) { + } else if (std::filesystem::is_directory(p, dir_ec)) { dir = p; base.clear(); } else { dir = p.parent_path(); base = p.filename().string(); if (dir.empty()) - dir = std::filesystem::current_path(); + dir = safe_current_path(); } std::error_code ec; @@ -2934,7 +2956,7 @@ cmd_newline(CommandContext &ctx) value = expand_user_path(value); // Ask before overwriting any existing file other than the // buffer's own (it used to ask only for unnamed buffers). - if (std::filesystem::exists(value) && !is_buffers_own_file(*buf, value)) { + if (fs_exists(value) && !is_buffers_own_file(*buf, value)) { ctx.editor.StartPrompt(Editor::PromptKind::Confirm, "Overwrite", ""); ctx.editor.SetPendingOverwritePath(value); ctx.editor.SetStatus( @@ -5555,6 +5577,27 @@ InstallDefaultCommands() } +// Run a command handler. An exception escaping a command (a filesystem +// error, bad_alloc on a huge operation, ...) used to reach main() and exit +// the editor, losing every unsaved buffer; report it and keep running. +static bool +run_handler(Editor &ed, const Command &cmd, CommandContext &ctx) +{ + if (!cmd.handler) + return false; + try { + return cmd.handler(ctx); + } catch (const std::exception &e) { + kte::ErrorHandler::Instance().Error("Command", std::string(cmd.name) + ": " + e.what(), ""); + ed.SetStatus(std::string("Error in ") + cmd.name + ": " + e.what()); + } catch (...) { + kte::ErrorHandler::Instance().Error("Command", std::string(cmd.name) + ": unknown exception", ""); + ed.SetStatus(std::string("Error in ") + cmd.name); + } + return false; +} + + bool Execute(Editor &ed, CommandId id, const std::string &arg, int count) { @@ -5605,7 +5648,7 @@ Execute(Editor &ed, CommandId id, const std::string &arg, int count) } CommandContext ctx{ed, arg, final_count}; - return cmd->handler ? cmd->handler(ctx) : false; + return run_handler(ed, *cmd, ctx); } @@ -5623,5 +5666,5 @@ Execute(Editor &ed, const std::string &name, const std::string &arg, int count) } } CommandContext ctx{ed, arg, count}; - return cmd->handler ? cmd->handler(ctx) : false; + return run_handler(ed, *cmd, ctx); } \ No newline at end of file diff --git a/tests/test_audit_regressions.cc b/tests/test_audit_regressions.cc index c106b38..d932782 100644 --- a/tests/test_audit_regressions.cc +++ b/tests/test_audit_regressions.cc @@ -831,3 +831,52 @@ TEST(Audit_HorizontalScroll_ControlCharsAreTwoCells) ASSERT_TRUE(h.Exec(CommandId::MoveEnd)); ASSERT_EQ(b.Coloffs(), (std::size_t) (100 - 80 + 1)); } + + +// Opening a directory or a FIFO fails cleanly (a directory used to throw +// bad_alloc and exit the editor; a FIFO blocked forever). +TEST(Audit_Open_NonRegularFiles_Rejected) +{ + TempDir d("open_nonreg"); + Buffer b; + std::string err; + ASSERT_TRUE(!b.OpenFromFile(d.path.string(), err)); + ASSERT_TRUE(err.find("directory") != std::string::npos); + const std::string fifo = (d.path / "pipe").string(); + ASSERT_EQ(::mkfifo(fifo.c_str(), 0600), 0); + err.clear(); + ASSERT_TRUE(!b.OpenFromFile(fifo, err)); +} + + +// Saving never replaces a non-regular file (e.g. a device node or FIFO). +TEST(Audit_Save_RefusesNonRegularTarget) +{ + TempDir d("save_nonreg"); + const std::string fifo = (d.path / "pipe").string(); + ASSERT_EQ(::mkfifo(fifo.c_str(), 0600), 0); + Buffer b; + b.insert_text(0, 0, "x\n"); + std::string err; + ASSERT_TRUE(!b.SaveAs(fifo, err)); + struct stat st{}; + ASSERT_EQ(::lstat(fifo.c_str(), &st), 0); + ASSERT_TRUE(S_ISFIFO(st.st_mode)); +} + + +// Filesystem errors in commands (here: a name longer than NAME_MAX) are +// reported, not thrown out of the editor. +TEST(Audit_Commands_FilesystemErrorsDoNotThrow) +{ + TestHarness h; + Editor &ed = h.EditorRef(); + h.Buf().insert_text(0, 0, "precious\n"); + const std::string longname = "/tmp/" + std::string(400, 'n'); + ASSERT_TRUE(Execute(ed, "save-as", longname) || true); + ASSERT_TRUE(h.Exec(CommandId::OpenFileStart)); + ASSERT_TRUE(h.Exec(CommandId::InsertText, longname)); + (void) h.Exec(CommandId::Newline); + (void) ed.ProcessPendingOpens(); + ASSERT_EQ(h.Text(), std::string("precious\n")); +}