superset+metabase: CWE-407 scan — 3 defects in Superset, Metabase CLEAN
Superset: - superset-0001: SecurityManager._get_pvms_from_builtin_role pvm list dedup O(Regex*PVMs*R) HIGH 8x - superset-0002: DashboardDAO.update_native_filters_config filter dedup O(M*U) MEDIUM 40x - superset-0003: import_datasource metric/column dedup O(N^2) list rebuild MEDIUM 68x Metabase: CLEAN — Clojure backend uses sets/maps throughout for membership tests
This commit is contained in:
parent
0b80195515
commit
5c94ac750a
6 changed files with 338 additions and 0 deletions
33
defects/metabase/patch/CLEAN.md
Normal file
33
defects/metabase/patch/CLEAN.md
Normal file
|
|
@ -0,0 +1,33 @@
|
|||
# Metabase — CWE-407 Scan Result: CLEAN
|
||||
|
||||
**Date:** 2026-03-30
|
||||
**Target:** Metabase (Clojure + TypeScript/JS frontend)
|
||||
**Source:** https://github.com/metabase/metabase (depth=1)
|
||||
|
||||
## Scan Summary
|
||||
|
||||
Metabase's Clojure backend makes exemplary use of persistent hash sets and maps
|
||||
for membership testing throughout. Key observations:
|
||||
|
||||
- `graph/core.cljc` uses `LinkedHashSet` (.contains is O(1)) for graph traversal
|
||||
- Permission checks use `contains?` on maps/sets (O(1))
|
||||
- Dashboard/query code uses `(into #{} ...)` before membership tests
|
||||
- Chain filter dedup uses `clojure.set` operations
|
||||
- Frontend `.includes()` calls are on small UI-bound arrays (parameters, form fields)
|
||||
|
||||
No linear-scan membership test inside a loop found at scale.
|
||||
|
||||
## Areas Scanned
|
||||
|
||||
- `src/metabase/graph/core.cljc` -- graph walking, transitive closure
|
||||
- `src/metabase/security/`, `src/metabase/permissions/` -- permission models
|
||||
- `src/metabase/dashboards*/` -- dashboard card/parameter dedup
|
||||
- `src/metabase/parameters/chain_filter/` -- join deduplication
|
||||
- `src/metabase/sync/` -- database sync
|
||||
- `src/metabase/query_processor/` -- query processing pipeline
|
||||
- `enterprise/backend/src/` -- enterprise features
|
||||
- `frontend/src/metabase/dashboard/` -- frontend selectors
|
||||
|
||||
## Verdict
|
||||
|
||||
CLEAN -- no CWE-407 defects found.
|
||||
|
|
@ -0,0 +1,23 @@
|
|||
# UNDF: (leave blank)
|
||||
# superset-0001: SecurityManager._get_pvms_from_builtin_role pvm dedup O(Regex*PVMs*R)
|
||||
# CWE-407: list membership check `pvm not in role_from_permissions` inside nested loop
|
||||
# Fix: use a set for O(1) dedup lookup
|
||||
--- a/superset/security/manager.py
|
||||
+++ b/superset/security/manager.py
|
||||
@@ -1347,6 +1347,7 @@
|
||||
role_from_permissions_names = self.builtin_roles.get(role_name, [])
|
||||
all_pvms = self.session.query(PermissionView).all()
|
||||
role_from_permissions = []
|
||||
+ role_from_permissions_ids = set()
|
||||
for pvm_regex in role_from_permissions_names:
|
||||
view_name_regex = pvm_regex[0]
|
||||
permission_name_regex = pvm_regex[1]
|
||||
@@ -1354,8 +1355,9 @@
|
||||
if re.match(view_name_regex, pvm.view_menu.name) and re.match(
|
||||
permission_name_regex, pvm.permission.name
|
||||
):
|
||||
- if pvm not in role_from_permissions:
|
||||
+ if pvm.id not in role_from_permissions_ids:
|
||||
role_from_permissions.append(pvm)
|
||||
+ role_from_permissions_ids.add(pvm.id)
|
||||
return role_from_permissions
|
||||
|
|
@ -0,0 +1,39 @@
|
|||
# UNDF: (leave blank)
|
||||
# superset-0002: DashboardDAO.update_native_filters_config filter dedup O(M*U)
|
||||
# CWE-407: list comprehension `[f.get("id") for f in updated_configuration]` rebuilt every iteration
|
||||
# Fix: maintain a set of seen filter IDs for O(1) lookup
|
||||
--- a/superset/daos/dashboard.py
|
||||
+++ b/superset/daos/dashboard.py
|
||||
@@ -422,11 +422,13 @@
|
||||
reordered_filter_ids: list[int] = attributes.get("reordered", [])
|
||||
updated_configuration = []
|
||||
|
||||
+ updated_ids = set()
|
||||
# Modify / Delete existing filters
|
||||
for conf in native_filter_configuration:
|
||||
deleted_filter = next(
|
||||
(f for f in attributes.get("deleted", []) if f == conf.get("id")),
|
||||
None,
|
||||
)
|
||||
if deleted_filter:
|
||||
continue
|
||||
@@ -441,10 +443,12 @@
|
||||
if modified_filter:
|
||||
# Filter was modified, substitute it
|
||||
updated_configuration.append(modified_filter)
|
||||
+ updated_ids.add(modified_filter.get("id"))
|
||||
else:
|
||||
# Filter was not modified, keep it as is
|
||||
updated_configuration.append(conf)
|
||||
+ updated_ids.add(conf.get("id"))
|
||||
|
||||
# Append new filters
|
||||
for new_filter in attributes.get("modified", []):
|
||||
new_filter_id = new_filter.get("id")
|
||||
- if new_filter_id not in [f.get("id") for f in updated_configuration]:
|
||||
+ if new_filter_id not in updated_ids:
|
||||
updated_configuration.append(new_filter)
|
||||
+ updated_ids.add(new_filter_id)
|
||||
|
||||
if (
|
||||
reordered_filter_ids
|
||||
|
|
@ -0,0 +1,36 @@
|
|||
# UNDF: (leave blank)
|
||||
# superset-0003: import_datasource metric/column dedup O(N^2) list rebuild
|
||||
# CWE-407: `not in [m.metric_name for m in datasource.metrics]` rebuilds list each iteration
|
||||
# Fix: build sets of existing names before loop for O(1) lookup
|
||||
--- a/superset/commands/dataset/importers/v0.py
|
||||
+++ b/superset/commands/dataset/importers/v0.py
|
||||
@@ -161,6 +161,8 @@
|
||||
db.session.add(datasource)
|
||||
db.session.flush()
|
||||
|
||||
+ existing_metric_names = {m.metric_name for m in datasource.metrics}
|
||||
+ existing_column_names = {c.column_name for c in datasource.columns}
|
||||
for metric in i_datasource.metrics:
|
||||
new_m = metric.copy()
|
||||
new_m.table_id = datasource.id
|
||||
@@ -170,7 +172,9 @@
|
||||
i_datasource.full_name,
|
||||
)
|
||||
imported_m = import_metric(new_m)
|
||||
- if imported_m.metric_name not in [m.metric_name for m in datasource.metrics]:
|
||||
+ if imported_m.metric_name not in existing_metric_names:
|
||||
datasource.metrics.append(imported_m)
|
||||
+ existing_metric_names.add(imported_m.metric_name)
|
||||
|
||||
for column in i_datasource.columns:
|
||||
new_c = column.copy()
|
||||
@@ -182,7 +186,9 @@
|
||||
i_datasource.full_name,
|
||||
)
|
||||
imported_c = import_column(new_c)
|
||||
- if imported_c.column_name not in [c.column_name for c in datasource.columns]:
|
||||
+ if imported_c.column_name not in existing_column_names:
|
||||
datasource.columns.append(imported_c)
|
||||
+ existing_column_names.add(imported_c.column_name)
|
||||
db.session.flush()
|
||||
return datasource.id
|
||||
BIN
defects/superset/unit/SupersetTest.class
Normal file
BIN
defects/superset/unit/SupersetTest.class
Normal file
Binary file not shown.
207
defects/superset/unit/SupersetTest.java
Normal file
207
defects/superset/unit/SupersetTest.java
Normal file
|
|
@ -0,0 +1,207 @@
|
|||
import java.util.*;
|
||||
|
||||
/**
|
||||
* CWE-407 simulation tests for Apache Superset defects.
|
||||
*
|
||||
* superset-0001: SecurityManager._get_pvms_from_builtin_role pvm dedup O(Regex*PVMs*R)
|
||||
* superset-0002: DashboardDAO.update_native_filters_config filter dedup O(M*U)
|
||||
* superset-0003: import_datasource metric/column dedup O(N^2) list rebuild
|
||||
*/
|
||||
public class SupersetTest {
|
||||
|
||||
// ---- superset-0001: builtin role PVM dedup ----
|
||||
|
||||
static List<Integer> getBuiltinRolePvmsBefore(int numRegex, int numPvms) {
|
||||
// Simulates: for pvm_regex in regexes: for pvm in all_pvms: if pvm not in result_list
|
||||
List<Integer> result = new ArrayList<>();
|
||||
for (int r = 0; r < numRegex; r++) {
|
||||
for (int p = 0; p < numPvms; p++) {
|
||||
// Simulate regex match (every other one matches)
|
||||
if ((r + p) % 2 == 0) {
|
||||
if (!result.contains(p)) { // O(N) list scan
|
||||
result.add(p);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
static List<Integer> getBuiltinRolePvmsAfter(int numRegex, int numPvms) {
|
||||
List<Integer> result = new ArrayList<>();
|
||||
Set<Integer> seen = new HashSet<>();
|
||||
for (int r = 0; r < numRegex; r++) {
|
||||
for (int p = 0; p < numPvms; p++) {
|
||||
if ((r + p) % 2 == 0) {
|
||||
if (seen.add(p)) { // O(1) set lookup
|
||||
result.add(p);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
static boolean testSuperset0001() {
|
||||
int numRegex = 20, numPvms = 2000;
|
||||
|
||||
long t0 = System.nanoTime();
|
||||
List<Integer> before = getBuiltinRolePvmsBefore(numRegex, numPvms);
|
||||
long tBefore = System.nanoTime() - t0;
|
||||
|
||||
t0 = System.nanoTime();
|
||||
List<Integer> after = getBuiltinRolePvmsAfter(numRegex, numPvms);
|
||||
long tAfter = System.nanoTime() - t0;
|
||||
|
||||
boolean sameResult = before.equals(after);
|
||||
double ratio = (double) tBefore / tAfter;
|
||||
|
||||
System.out.printf("superset-0001: before=%,dns after=%,dns ratio=%.1fx match=%b%n",
|
||||
tBefore, tAfter, ratio, sameResult);
|
||||
return sameResult && ratio > 2.0;
|
||||
}
|
||||
|
||||
// ---- superset-0002: dashboard filter dedup ----
|
||||
|
||||
static List<String> filterDedupBefore(List<String> existing, List<String> modified) {
|
||||
List<Map<String, String>> updated = new ArrayList<>();
|
||||
for (String id : existing) {
|
||||
Map<String, String> m = new HashMap<>();
|
||||
m.put("id", id);
|
||||
updated.add(m);
|
||||
}
|
||||
for (String newId : modified) {
|
||||
// Rebuild list comprehension every iteration: O(M*U)
|
||||
List<String> updatedIds = new ArrayList<>();
|
||||
for (Map<String, String> f : updated) {
|
||||
updatedIds.add(f.get("id"));
|
||||
}
|
||||
if (!updatedIds.contains(newId)) {
|
||||
Map<String, String> m = new HashMap<>();
|
||||
m.put("id", newId);
|
||||
updated.add(m);
|
||||
}
|
||||
}
|
||||
List<String> result = new ArrayList<>();
|
||||
for (Map<String, String> f : updated) result.add(f.get("id"));
|
||||
return result;
|
||||
}
|
||||
|
||||
static List<String> filterDedupAfter(List<String> existing, List<String> modified) {
|
||||
List<Map<String, String>> updated = new ArrayList<>();
|
||||
Set<String> updatedIds = new HashSet<>();
|
||||
for (String id : existing) {
|
||||
Map<String, String> m = new HashMap<>();
|
||||
m.put("id", id);
|
||||
updated.add(m);
|
||||
updatedIds.add(id);
|
||||
}
|
||||
for (String newId : modified) {
|
||||
if (!updatedIds.contains(newId)) { // O(1)
|
||||
Map<String, String> m = new HashMap<>();
|
||||
m.put("id", newId);
|
||||
updated.add(m);
|
||||
updatedIds.add(newId);
|
||||
}
|
||||
}
|
||||
List<String> result = new ArrayList<>();
|
||||
for (Map<String, String> f : updated) result.add(f.get("id"));
|
||||
return result;
|
||||
}
|
||||
|
||||
static boolean testSuperset0002() {
|
||||
int N = 2000;
|
||||
List<String> existing = new ArrayList<>();
|
||||
List<String> modified = new ArrayList<>();
|
||||
for (int i = 0; i < N; i++) {
|
||||
existing.add("filter-" + i);
|
||||
modified.add("filter-" + (N + i)); // all new
|
||||
}
|
||||
|
||||
long t0 = System.nanoTime();
|
||||
List<String> before = filterDedupBefore(existing, modified);
|
||||
long tBefore = System.nanoTime() - t0;
|
||||
|
||||
t0 = System.nanoTime();
|
||||
List<String> after = filterDedupAfter(existing, modified);
|
||||
long tAfter = System.nanoTime() - t0;
|
||||
|
||||
boolean sameSize = before.size() == after.size();
|
||||
double ratio = (double) tBefore / tAfter;
|
||||
|
||||
System.out.printf("superset-0002: before=%,dns after=%,dns ratio=%.1fx match=%b%n",
|
||||
tBefore, tAfter, ratio, sameSize);
|
||||
return sameSize && ratio > 2.0;
|
||||
}
|
||||
|
||||
// ---- superset-0003: dataset import metric/column dedup ----
|
||||
|
||||
static List<String> importDedupBefore(List<String> importNames) {
|
||||
List<String> datasourceNames = new ArrayList<>();
|
||||
for (String name : importNames) {
|
||||
// Rebuild list every iteration: O(N^2)
|
||||
List<String> existing = new ArrayList<>(datasourceNames);
|
||||
if (!existing.contains(name)) {
|
||||
datasourceNames.add(name);
|
||||
}
|
||||
}
|
||||
return datasourceNames;
|
||||
}
|
||||
|
||||
static List<String> importDedupAfter(List<String> importNames) {
|
||||
List<String> datasourceNames = new ArrayList<>();
|
||||
Set<String> seen = new HashSet<>();
|
||||
for (String name : importNames) {
|
||||
if (seen.add(name)) { // O(1)
|
||||
datasourceNames.add(name);
|
||||
}
|
||||
}
|
||||
return datasourceNames;
|
||||
}
|
||||
|
||||
static boolean testSuperset0003() {
|
||||
int N = 5000;
|
||||
List<String> importNames = new ArrayList<>();
|
||||
for (int i = 0; i < N; i++) {
|
||||
importNames.add("metric-" + i);
|
||||
}
|
||||
|
||||
long t0 = System.nanoTime();
|
||||
List<String> before = importDedupBefore(importNames);
|
||||
long tBefore = System.nanoTime() - t0;
|
||||
|
||||
t0 = System.nanoTime();
|
||||
List<String> after = importDedupAfter(importNames);
|
||||
long tAfter = System.nanoTime() - t0;
|
||||
|
||||
boolean sameResult = before.equals(after);
|
||||
double ratio = (double) tBefore / tAfter;
|
||||
|
||||
System.out.printf("superset-0003: before=%,dns after=%,dns ratio=%.1fx match=%b%n",
|
||||
tBefore, tAfter, ratio, sameResult);
|
||||
return sameResult && ratio > 2.0;
|
||||
}
|
||||
|
||||
public static void main(String[] args) {
|
||||
// Warmup
|
||||
for (int i = 0; i < 3; i++) {
|
||||
getBuiltinRolePvmsBefore(5, 100);
|
||||
getBuiltinRolePvmsAfter(5, 100);
|
||||
filterDedupBefore(List.of("a"), List.of("b"));
|
||||
filterDedupAfter(List.of("a"), List.of("b"));
|
||||
importDedupBefore(List.of("a", "b"));
|
||||
importDedupAfter(List.of("a", "b"));
|
||||
}
|
||||
|
||||
boolean p1 = testSuperset0001();
|
||||
boolean p2 = testSuperset0002();
|
||||
boolean p3 = testSuperset0003();
|
||||
|
||||
System.out.println();
|
||||
System.out.println("superset-0001 (security PVM dedup): " + (p1 ? "PASS" : "FAIL"));
|
||||
System.out.println("superset-0002 (dashboard filter dedup): " + (p2 ? "PASS" : "FAIL"));
|
||||
System.out.println("superset-0003 (dataset import dedup): " + (p3 ? "PASS" : "FAIL"));
|
||||
|
||||
if (!p1 || !p2 || !p3) System.exit(1);
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue