Merge branch 'master' into develop

Author: tstack

NEWS.md

@@ -92,8 +92,17 @@ Breaking changes:
   uses 1,000 for the base instead of 1,024.
 
 Bug Fixes:
+* A PRQL query can now start with `let` in interactive
+  mode.
 * Fix a bug in file loading that could cause a short
   read and crash in some situations.
+* Fix a lockup when viewing a file that contained log
+  messages and lots of binary data.
+* More hardening for some diabolical inputs:
+  - Unsupported escape-sequences were ignored before,
+    but they are displayed now.
+  - Checks for archives with file paths that could
+    escape containment.
 
 
 ## lnav v0.14.0

src/CMakeLists.txt

@@ -366,7 +366,6 @@ add_custom_command(
 list(APPEND GEN_SRCS builtin-scripts.h builtin-scripts.cc)
 
 set(BUILTIN_SH_SCRIPTS
-        scripts/com.vmware.btresolver.py
         scripts/dump-pid.sh
         scripts/pcap_log-converter.sh
         scripts/zookeeper.sql

src/archive_manager.cc

@@ -302,6 +302,7 @@ extract(const std::string& filename, const extract_cb& cb)
                                filename,
                                archive_error_string(arc)));
     }
+    auto tmp_base = tmp_path.lexically_normal();
 
     log_info("extracting %s to %s", filename.c_str(), tmp_path.c_str());
     while (true) {
@@ -332,7 +333,12 @@ extract(const std::string& filename, const extract_cb& cb)
         if (strcmp(format_name, "raw") == 0 && filter_count >= 2) {
             desired_pathname = fs::path(filename).filename();
         }
-        auto entry_path = tmp_path / desired_pathname;
+        auto entry_path = (tmp_path / desired_pathname).lexically_normal();
+        auto rel = entry_path.lexically_relative(tmp_base);
+        if (rel.empty() || lnav::filesystem::contains_dotdot(rel)) {
+            log_warning("ignoring naughty path: %s", entry_path_str);
+            continue;
+        }
         auto* prog = cb(
             entry_path,
             archive_entry_size_is_set(entry) ? archive_entry_size(entry) : -1);
@@ -341,6 +347,44 @@ extract(const std::string& filename, const extract_cb& cb)
 
         archive_entry_set_perm(
             wentry, S_IRUSR | (S_ISDIR(entry_mode) ? S_IXUSR | S_IWUSR : 0));
+        if (S_ISLNK(entry_mode)) {
+            auto* target_path_str = archive_entry_symlink(wentry);
+            if (target_path_str == nullptr) {
+                log_warning("symlink is null: %s", entry_path_str);
+                continue;
+            }
+            auto link_target = fs::path(target_path_str);
+            if (link_target.is_absolute()) {
+                // Confine to tmp_path: strip root, rejoin under tmp_path,
+                // then express as relative from the symlink's own directory.
+                auto confined = (tmp_path / link_target.relative_path())
+                                    .lexically_normal();
+                auto rewritten
+                    = confined.lexically_relative(entry_path.parent_path());
+                if (rewritten.empty()) {
+                    log_warning(
+                        "ignoring symlink with unrepresentable target: %s",
+                        entry_path_str);
+                    continue;
+                }
+                archive_entry_set_symlink(wentry, rewritten.c_str());
+            } else {
+                // Relative target: verify it lands inside tmp_base, but leave
+                // the literal target alone so kernel resolution matches.
+                auto resolved = (entry_path.parent_path() / link_target)
+                                    .lexically_normal();
+                auto target_rel = resolved.lexically_relative(tmp_base);
+                if (target_rel.empty()
+                    || lnav::filesystem::contains_dotdot(target_rel))
+                {
+                    log_warning(
+                        "ignoring naughty symlink '%s' with target '%s'",
+                        entry_path_str,
+                        target_path_str);
+                    continue;
+                }
+            }
+        }
         r = archive_write_header(ext, wentry);
         if (r < ARCHIVE_OK) {
             return Err(

src/base/ansi_scrubber.cc

@@ -40,11 +40,20 @@
 #include "pcrepp/pcre2pp.hh"
 #include "scn/scan.h"
 
+using std::string_literals::operator""s;
+
 static const lnav::pcre2pp::code&
 ansi_regex()
 {
     static const auto retval = lnav::pcre2pp::code::from_const(
-        R"(\x00|\x1b\[([\d=;:\?]*)([a-zA-Z])|\x1b\](\d+);(.*?)(?:\x07|\x1b\\)|(?:\X\x08\X)+|(\x16+))");
+        R"(
+            \x00                                      # NUL
+          | \x1b \[ ([\d=;:\?]*) ([a-zA-Z])           # CSI: ESC [ <params> <final>
+          | \x1b \] (\d+) ; (.*?) (?:\x07 | \x1b\\)   # OSC: ESC ] <code> ; <payload> (BEL|ST)
+          | (?: \X \x08 \X )+                         # Overstrike: glyph BS glyph
+          | (\x16+)                                   # SYN run
+        )",
+        PCRE2_EXTENDED);
 
     return retval;
 }
@@ -290,7 +299,7 @@ scrub_ansi_string(std::string& str, string_attrs_t* sa)
 
             if (osc_id) {
                 switch (osc_id->value()) {
-                    case 8:
+                    case 8: {
                         auto split_res = md[4]->split_pair(semi_pred);
                         if (split_res) {
                             // auto params = split_res->first;
@@ -313,6 +322,20 @@ scrub_ansi_string(std::string& str, string_attrs_t* sa)
                             }
                         }
                         break;
+                    }
+                    default: {
+                        if (sa != nullptr) {
+                            lr.lr_start = cp_dst;
+                            lr.lr_end = cp_dst;
+                            tmp_sa.emplace_back(
+                                lr,
+                                SA_UNSUPPORTED.value(fmt::format(
+                                    FMT_STRING("ANSI sequence: OSC {} {}"),
+                                    md[3],
+                                    md[4])));
+                        }
+                        break;
+                    }
                 }
             }
         } else if (md[1]) {
@@ -468,6 +491,19 @@ scrub_ansi_string(std::string& str, string_attrs_t* sa)
                     }
                     break;
                 }
+                default: {
+                    if (sa != nullptr) {
+                        lr.lr_start = cp_dst;
+                        lr.lr_end = cp_dst;
+                        tmp_sa.emplace_back(
+                            lr,
+                            SA_UNSUPPORTED.value(fmt::format(
+                                FMT_STRING("ANSI sequence: ESC [ {} {}"),
+                                seq,
+                                terminator)));
+                    }
+                    break;
+                }
             }
         }
         if (md[1] || md[3] || md[5]) {
@@ -485,6 +521,7 @@ scrub_ansi_string(std::string& str, string_attrs_t* sa)
                         sa.sa_range.lr_end = cp_dst;
                         if (sa.sa_range.empty()) {
                             sa.sa_type = &SA_INVALID;
+                            sa.sa_value = "zero-length attribute"s;
                         }
                     }
                     if (sf.sf_end < str.size()) {

src/base/color_spaces.cc

@@ -324,14 +324,28 @@ lab_color::readable(const lab_color& bg) const
     constexpr double CONTRAST_BOOST = 20.0;
     constexpr double CHROMA_HEADROOM = 30.0;
 
+    constexpr double L_FLOOR = 25.0;
+    constexpr double L_CEIL = 90.0;
+
     bool changed = false;
     double new_l = this->lc_l;
     if (std::abs(this->lc_l - bg.lc_l) < CONTRAST_BOOST) {
+        // Push fg away from bg by 2x the boost in fg's current direction;
+        // if that side is out of headroom against [L_FLOOR, L_CEIL], flip
+        // to the other side instead.  Targets are pre-clamped so that when
+        // both sides are blocked we still land on a valid L*.
         const double delta = CONTRAST_BOOST * 2.0;
-        const bool go_up = (this->lc_l > bg.lc_l) ? (bg.lc_l + delta <= 90.0)
-                                                  : (bg.lc_l - delta < 25.0);
-        new_l = go_up ? std::min(bg.lc_l + delta, 90.0)
-                      : std::max(bg.lc_l - delta, 25.0);
+        const double up_target = std::min(bg.lc_l + delta, L_CEIL);
+        const double down_target = std::max(bg.lc_l - delta, L_FLOOR);
+        const bool up_has_room = bg.lc_l + delta <= L_CEIL;
+        const bool down_has_room = bg.lc_l - delta >= L_FLOOR;
+        const bool prefer_up = this->lc_l > bg.lc_l;
+
+        if (prefer_up) {
+            new_l = up_has_room ? up_target : down_target;
+        } else {
+            new_l = down_has_room ? down_target : up_target;
+        }
         changed = true;
     }
 

src/base/fs_util.cc

@@ -196,6 +196,18 @@ escape_path(const std::filesystem::path& p, path_type pt)
     return retval;
 }
 
+bool
+contains_dotdot(const std::filesystem::path& p)
+{
+    for (const auto& part : p) {
+        if (part == "..") {
+            return true;
+        }
+    }
+
+    return false;
+}
+
 bool
 is_url(const std::string& fn)
 {

src/base/fs_util.hh

@@ -79,6 +79,8 @@ enum class path_type {
 std::string escape_path(const std::filesystem::path& p,
                         path_type pt = path_type::normal);
 
+bool contains_dotdot(const std::filesystem::path& p);
+
 path_type determine_path_type(const std::string& arg);
 
 struct path_transcoder {

src/base/fs_util.tests.cc

@@ -100,3 +100,13 @@ TEST_CASE("fs_util::escape_path")
 
     CHECK("\\$abc" == lnav::filesystem::escape_path(p2));
 }
+
+TEST_CASE("fs_util::contains_dotdot")
+{
+    CHECK_FALSE(
+        lnav::filesystem::contains_dotdot(std::filesystem::path{"/abc/def"}));
+    CHECK(lnav::filesystem::contains_dotdot(
+        std::filesystem::path{"/abc/../def"}));
+    CHECK_FALSE(lnav::filesystem::contains_dotdot(
+        std::filesystem::path{"/abc..def"}));
+}

src/base/intern_string.hh

@@ -1023,6 +1023,20 @@ struct formatter<std::error_code> : formatter<string_view> {
         return formatter<string_view>::format(ec.message(), ctx);
     }
 };
+
+template<typename I>
+struct formatter<std::optional<I>> : formatter<I> {
+    template<typename FormatContext>
+    auto format(const std::optional<I>& opt, FormatContext& ctx) const
+    {
+        if (!opt) {
+            return formatter<string_view>::format("\u2205", ctx);
+        }
+
+        return formatter<I>::format(opt.value(), ctx);
+    }
+};
+
 }  // namespace fmt
 
 namespace std {

src/base/lnav.console.cc

@@ -633,6 +633,8 @@ println(FILE* file, const attr_line_t& al)
                     if (color_opt) {
                         fg_style = fmt::fg(color_opt.value());
                     }
+                } else if (attr.sa_type == &SA_UNSUPPORTED) {
+                    line_style |= fmt::fg(fmt::terminal_color::yellow);
                 } else if (attr.sa_type == &VC_STYLE) {
                     auto saw = string_attr_wrapper<text_attrs>(&attr);
                     auto style = saw.get();