Merge pull request #2882 from finos/fix-filter-uaf

Fix memory corruption on filter text overflow
finos · Dec 21, 2024 · 74df6a2 · 74df6a2
2 parents 9b6ea8c + d5b63b6
commit 74df6a2
Show file tree

Hide file tree

Showing 4 changed files with 126 additions and 4 deletions.
diff --git a/cpp/perspective/src/cpp/scalar.cpp b/cpp/perspective/src/cpp/scalar.cpp
@@ -1628,6 +1628,11 @@ t_tscalar::can_store_inplace(const char* s) {
     return strlen(s) + 1 <= static_cast<size_t>(SCALAR_INPLACE_LEN);
 }
 
+bool
+t_tscalar::can_store_inplace(const std::string& s) {
+    return s.size() + 1 <= static_cast<size_t>(SCALAR_INPLACE_LEN);
+}
+
 bool
 t_tscalar::is_nan() const {
     if (m_type == DTYPE_FLOAT64) {

diff --git a/cpp/perspective/src/cpp/server.cpp b/cpp/perspective/src/cpp/server.cpp
@@ -1632,8 +1632,29 @@ ProtoServer::_handle_request(std::uint32_t client_id, const Request& req) {
             std::vector<
                 std::tuple<std::string, std::string, std::vector<t_tscalar>>>
                 filter;
+            filter.reserve(cfg.filter().size());
+
+            for (const auto& f : cfg.filter()) {
+                for (const auto& arg : f.value()) {
+                    switch (arg.scalar_case()) {
+                        case proto::Scalar::kString: {
+                            if (!t_tscalar::can_store_inplace(arg.string())) {
+                                vocab.get_interned(arg.string());
+                            }
+                            break;
+                        }
+                        case proto::Scalar::kBool:
+                        case proto::Scalar::kFloat:
+                        case proto::Scalar::kNull:
+                        case proto::Scalar::SCALAR_NOT_SET:
+                            break;
+                    }
+                }
+            }
+
             for (const auto& f : cfg.filter()) {
                 std::vector<t_tscalar> args;
+                args.reserve(f.value().size());
                 for (const auto& arg : f.value()) {
                     t_tscalar a;
                     a.clear();
@@ -1658,10 +1679,7 @@ ProtoServer::_handle_request(std::uint32_t client_id, const Request& req) {
                                 );
                             }
 
-                            if (!t_tscalar::can_store_inplace(
-                                    arg.string().c_str()
-                                )) {
-
+                            if (!t_tscalar::can_store_inplace(arg.string())) {
                                 a = coerce_to(
                                     schema->get_dtype(f.column()),
                                     vocab.unintern_c(

diff --git a/cpp/perspective/src/include/perspective/scalar.h b/cpp/perspective/src/include/perspective/scalar.h
@@ -189,6 +189,7 @@ struct PERSPECTIVE_EXPORT t_tscalar {
     const char* get_char_ptr() const;
     bool is_inplace() const;
     static bool can_store_inplace(const char* s);
+    static bool can_store_inplace(const std::string& s);
 
     template <typename DATA_T>
     t_tscalar coerce_numeric() const;

diff --git a/rust/perspective-js/test/js/filters.spec.js b/rust/perspective-js/test/js/filters.spec.js
@@ -503,6 +503,104 @@ const datetime_data_local = [
             });
         });
 
+        test.describe("long strings", function () {
+            test("", async function () {
+                const data = [
+                    {
+                        field: 100,
+                        index: "1",
+                        title: "s15",
+                        ts: "2024-11-30T06:50:10.214Z",
+                    },
+                    {
+                        field: 100,
+                        index: "2",
+                        title: "sys_c_c 15min",
+                        ts: "2024-11-30T07:06:00.763Z",
+                    },
+                    {
+                        field: 100,
+                        index: "3",
+                        title: "s15",
+                        ts: "2024-12-01T06:50:15.596Z",
+                    },
+                    {
+                        field: 100,
+                        index: "4",
+                        title: "sys_c_c 15min",
+                        ts: "2024-12-01T07:06:04.909Z",
+                    },
+                    {
+                        field: 100,
+                        index: "5",
+                        title: "s15",
+                        ts: "2024-12-02T06:50:24.712Z",
+                    },
+                    {
+                        field: 100,
+                        index: "6",
+                        title: "sys_c_c 15min",
+                        ts: "2024-12-02T07:06:15.339Z",
+                    },
+                    {
+                        field: 100,
+                        index: "7",
+                        title: "s15",
+                        ts: "2024-12-03T06:50:22.144Z",
+                    },
+                    {
+                        field: 100,
+                        index: "8",
+                        title: "sys_c_c 15min",
+                        ts: "2024-12-03T07:06:20.146Z",
+                    },
+                ];
+
+                const config = {
+                    group_by: ["ts"],
+                    split_by: [],
+                    columns: ["field"],
+                    filter: [
+                        ["title", "in", ["sys_c_c 15min"]],
+                        ["ts", ">=", "2024-12-01T00:00:00.000Z"],
+                        ["ts", "<=", "2024-12-02T23:59:59.999Z"],
+                    ],
+                    expressions: {},
+                    aggregates: {},
+                };
+
+                const table = await perspective.table(
+                    {
+                        title: "string",
+                        field: "float",
+                        ts: "datetime",
+                        index: "string",
+                    },
+                    { index: "index" }
+                );
+
+                let x, y;
+                let xx = new Promise((xxx) => {
+                    x = xxx;
+                });
+                let yy = new Promise((yyy) => {
+                    y = yyy;
+                });
+
+                await table.view(config).then((view) => {
+                    view.on_update(() => view.to_json().then(x));
+                });
+
+                await table.view(config).then((view) => {
+                    view.on_update(() => view.to_json().then(y));
+                });
+
+                await table.update(data);
+                const [xxx, yyy] = await Promise.all([xx, yy]);
+                expect(xxx).toEqual(yyy);
+            });
+        });
+
         test.describe("is null", function () {
             test("returns the correct null cells for string column", async function () {
                 const table = await perspective.table([