cockroach/influxdb: CWE-407 findings
This commit is contained in:
parent
95648fb2b0
commit
37836b498d
9 changed files with 546 additions and 0 deletions
|
|
@ -0,0 +1,38 @@
|
|||
# UNDF:
|
||||
--- a/pkg/sql/opt/exec/execbuilder/builder.go
|
||||
+++ b/pkg/sql/opt/exec/execbuilder/builder.go
|
||||
@@ -7,7 +7,6 @@ package execbuilder
|
||||
import (
|
||||
"context"
|
||||
- "slices"
|
||||
"strconv"
|
||||
"time"
|
||||
|
||||
@@ -197,14 +197,20 @@ type IndexesUsed struct {
|
||||
indexes []struct {
|
||||
tableID cat.StableID
|
||||
indexID cat.StableID
|
||||
}
|
||||
+ seen map[[2]cat.StableID]struct{}
|
||||
}
|
||||
|
||||
// add adds the given index to the list, if it is not already present.
|
||||
func (iu *IndexesUsed) add(tableID, indexID cat.StableID) {
|
||||
- s := struct {
|
||||
- tableID cat.StableID
|
||||
- indexID cat.StableID
|
||||
- }{tableID, indexID}
|
||||
- if !slices.Contains(iu.indexes, s) {
|
||||
- iu.indexes = append(iu.indexes, s)
|
||||
+ key := [2]cat.StableID{tableID, indexID}
|
||||
+ if iu.seen == nil {
|
||||
+ iu.seen = make(map[[2]cat.StableID]struct{})
|
||||
+ }
|
||||
+ if _, ok := iu.seen[key]; !ok {
|
||||
+ iu.seen[key] = struct{}{}
|
||||
+ iu.indexes = append(iu.indexes, struct {
|
||||
+ tableID cat.StableID
|
||||
+ indexID cat.StableID
|
||||
+ }{tableID, indexID})
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,34 @@
|
|||
# UNDF:
|
||||
--- a/pkg/sql/show_fingerprints.go
|
||||
+++ b/pkg/sql/show_fingerprints.go
|
||||
@@ -432,13 +432,17 @@ func BuildExperimentalFingerprintQueryForIndex(
|
||||
tableDesc catalog.TableDescriptor, index catalog.Index, ignoredColumns []string,
|
||||
) (string, error) {
|
||||
+ ignored := make(map[string]struct{}, len(ignoredColumns))
|
||||
+ for _, c := range ignoredColumns {
|
||||
+ ignored[c] = struct{}{}
|
||||
+ }
|
||||
cols := make([]string, 0, len(tableDesc.PublicColumns()))
|
||||
var numBytesCols int
|
||||
addColumn := func(col catalog.Column) {
|
||||
- if slices.Contains(ignoredColumns, col.GetName()) {
|
||||
+ if _, skip := ignored[col.GetName()]; skip {
|
||||
return
|
||||
}
|
||||
// rest unchanged
|
||||
@@ -502,12 +506,16 @@ func BuildFingerprintQueryForIndex(
|
||||
tableDesc catalog.TableDescriptor, index catalog.Index, ignoredColumns []string,
|
||||
) (string, error) {
|
||||
+ ignored := make(map[string]struct{}, len(ignoredColumns))
|
||||
+ for _, c := range ignoredColumns {
|
||||
+ ignored[c] = struct{}{}
|
||||
+ }
|
||||
cols := make([]string, 0, len(tableDesc.PublicColumns()))
|
||||
addColumn := func(col catalog.Column) {
|
||||
- if slices.Contains(ignoredColumns, col.GetName()) {
|
||||
+ if _, skip := ignored[col.GetName()]; skip {
|
||||
return
|
||||
}
|
||||
|
||||
cols = append(cols, makeColumnNameOrExpr(col))
|
||||
}
|
||||
|
|
@ -0,0 +1,20 @@
|
|||
# UNDF:
|
||||
--- a/pkg/sql/authorization.go
|
||||
+++ b/pkg/sql/authorization.go
|
||||
@@ -564,10 +564,14 @@ func EnsureUserOnlyBelongsToRoles(
|
||||
// Compute the differences between the current roles and the desired roles
|
||||
// to determine which roles need to granted or revoked.
|
||||
rolesToRevoke := make([]username.SQLUsername, 0, len(currentRoles))
|
||||
rolesToGrant := make([]username.SQLUsername, 0, len(roles))
|
||||
+ // Build a set for O(1) membership tests instead of O(D) slices.Contains.
|
||||
+ desiredSet := make(map[username.SQLUsername]struct{}, len(roles))
|
||||
+ for _, r := range roles {
|
||||
+ desiredSet[r] = struct{}{}
|
||||
+ }
|
||||
for role := range currentRoles {
|
||||
- if !slices.Contains(roles, role) {
|
||||
+ if _, ok := desiredSet[role]; !ok {
|
||||
rolesToRevoke = append(rolesToRevoke, role)
|
||||
}
|
||||
}
|
||||
for _, role := range roles {
|
||||
BIN
defects/cockroach/unit/CockroachTest.class
Normal file
BIN
defects/cockroach/unit/CockroachTest.class
Normal file
Binary file not shown.
204
defects/cockroach/unit/CockroachTest.java
Normal file
204
defects/cockroach/unit/CockroachTest.java
Normal file
|
|
@ -0,0 +1,204 @@
|
|||
import java.util.*;
|
||||
|
||||
/**
|
||||
* CWE-407 unit tests for CockroachDB — three defects:
|
||||
*
|
||||
* cockroach-0001: IndexesUsed.add() calls slices.Contains on a growing slice
|
||||
* for each of N add() calls → O(N²) index deduplication during
|
||||
* SQL query plan building.
|
||||
*
|
||||
* cockroach-0002: BuildFingerprintQueryForIndex / BuildExperimentalFingerprintQueryForIndex
|
||||
* call slices.Contains(ignoredColumns, col) for every column in
|
||||
* every index column loop → O(C × I) where C = columns, I = ignored list.
|
||||
*
|
||||
* cockroach-0003: EnsureUserOnlyBelongsToRoles iterates currentRoles (size R) and
|
||||
* calls slices.Contains(roles, role) (size D) for each → O(R × D)
|
||||
* during LDAP-driven role synchronisation.
|
||||
*/
|
||||
public class CockroachTest {
|
||||
|
||||
// ── cockroach-0001 ────────────────────────────────────────────────────────
|
||||
|
||||
/** Defective: ArrayList.contains inside an accumulation loop → O(N²). */
|
||||
static List<long[]> indexesUsedAdd_defective(int n) {
|
||||
List<long[]> indexes = new ArrayList<>();
|
||||
for (long i = 0; i < n; i++) {
|
||||
long tableID = i % 50;
|
||||
long indexID = i % 20;
|
||||
long[] entry = new long[]{tableID, indexID};
|
||||
boolean found = false;
|
||||
for (long[] e : indexes) {
|
||||
if (e[0] == entry[0] && e[1] == entry[1]) { found = true; break; }
|
||||
}
|
||||
if (!found) indexes.add(entry);
|
||||
}
|
||||
return indexes;
|
||||
}
|
||||
|
||||
/** Fixed: HashMap set for O(1) membership → O(N) total. */
|
||||
static List<long[]> indexesUsedAdd_fixed(int n) {
|
||||
List<long[]> indexes = new ArrayList<>();
|
||||
Set<Long> seen = new HashSet<>();
|
||||
for (long i = 0; i < n; i++) {
|
||||
long tableID = i % 50;
|
||||
long indexID = i % 20;
|
||||
long key = tableID * 1_000_000L + indexID;
|
||||
if (seen.add(key)) {
|
||||
indexes.add(new long[]{tableID, indexID});
|
||||
}
|
||||
}
|
||||
return indexes;
|
||||
}
|
||||
|
||||
// ── cockroach-0002 ────────────────────────────────────────────────────────
|
||||
|
||||
/** Defective: linear scan of ignoredColumns for every column → O(C × I). */
|
||||
static List<String> fingerprintColumns_defective(List<String> columns, List<String> ignored) {
|
||||
List<String> result = new ArrayList<>();
|
||||
for (String col : columns) {
|
||||
if (ignored.contains(col)) continue; // O(I) per column
|
||||
result.add(col);
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
/** Fixed: build a HashSet once, then O(1) per column → O(C + I). */
|
||||
static List<String> fingerprintColumns_fixed(List<String> columns, List<String> ignored) {
|
||||
Set<String> ignoredSet = new HashSet<>(ignored);
|
||||
List<String> result = new ArrayList<>();
|
||||
for (String col : columns) {
|
||||
if (!ignoredSet.contains(col)) result.add(col);
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
// ── cockroach-0003 ────────────────────────────────────────────────────────
|
||||
|
||||
/** Defective: for each currentRole call roles.contains → O(R × D). */
|
||||
static List<String> rolesToRevoke_defective(Set<String> currentRoles, List<String> desiredRoles) {
|
||||
List<String> toRevoke = new ArrayList<>();
|
||||
for (String role : currentRoles) {
|
||||
if (!desiredRoles.contains(role)) toRevoke.add(role); // O(D) per role
|
||||
}
|
||||
return toRevoke;
|
||||
}
|
||||
|
||||
/** Fixed: build desiredSet once → O(R + D). */
|
||||
static List<String> rolesToRevoke_fixed(Set<String> currentRoles, List<String> desiredRoles) {
|
||||
Set<String> desiredSet = new HashSet<>(desiredRoles);
|
||||
List<String> toRevoke = new ArrayList<>();
|
||||
for (String role : currentRoles) {
|
||||
if (!desiredSet.contains(role)) toRevoke.add(role);
|
||||
}
|
||||
return toRevoke;
|
||||
}
|
||||
|
||||
// ── helpers ───────────────────────────────────────────────────────────────
|
||||
|
||||
static long bench(Runnable r) {
|
||||
long t0 = System.nanoTime();
|
||||
r.run();
|
||||
return System.nanoTime() - t0;
|
||||
}
|
||||
|
||||
// ── main ─────────────────────────────────────────────────────────────────
|
||||
|
||||
public static void main(String[] args) {
|
||||
int pass = 0, fail = 0;
|
||||
|
||||
// --- cockroach-0001 correctness ---
|
||||
{
|
||||
List<long[]> def = indexesUsedAdd_defective(500);
|
||||
List<long[]> fix = indexesUsedAdd_fixed(500);
|
||||
if (def.size() == fix.size()) {
|
||||
System.out.println("PASS cockroach-0001 correctness (size=" + def.size() + ")");
|
||||
pass++;
|
||||
} else {
|
||||
System.out.println("FAIL cockroach-0001 correctness def=" + def.size() + " fix=" + fix.size());
|
||||
fail++;
|
||||
}
|
||||
}
|
||||
|
||||
// --- cockroach-0001 performance ---
|
||||
{
|
||||
int N = 2000;
|
||||
long tDef = bench(() -> indexesUsedAdd_defective(N));
|
||||
long tFix = bench(() -> indexesUsedAdd_fixed(N));
|
||||
double ratio = (double) tDef / Math.max(tFix, 1);
|
||||
System.out.printf("PASS cockroach-0001 perf defective=%dms fixed=%dms ratio=%.1fx%n",
|
||||
tDef / 1_000_000, tFix / 1_000_000, ratio);
|
||||
if (ratio >= 2.0) pass++; else { System.out.println("FAIL cockroach-0001 perf ratio too low"); fail++; }
|
||||
}
|
||||
|
||||
// --- cockroach-0002 correctness ---
|
||||
{
|
||||
List<String> columns = new ArrayList<>();
|
||||
for (int i = 0; i < 200; i++) columns.add("col_" + i);
|
||||
List<String> ignored = new ArrayList<>();
|
||||
for (int i = 0; i < 50; i++) ignored.add("col_" + (i * 4));
|
||||
|
||||
List<String> def = fingerprintColumns_defective(columns, ignored);
|
||||
List<String> fix = fingerprintColumns_fixed(columns, ignored);
|
||||
if (def.equals(fix)) {
|
||||
System.out.println("PASS cockroach-0002 correctness (kept=" + def.size() + ")");
|
||||
pass++;
|
||||
} else {
|
||||
System.out.println("FAIL cockroach-0002 correctness");
|
||||
fail++;
|
||||
}
|
||||
}
|
||||
|
||||
// --- cockroach-0002 performance ---
|
||||
{
|
||||
List<String> columns = new ArrayList<>();
|
||||
for (int i = 0; i < 1000; i++) columns.add("col_" + i);
|
||||
List<String> ignored = new ArrayList<>();
|
||||
for (int i = 0; i < 500; i++) ignored.add("col_" + (i * 2));
|
||||
|
||||
long tDef = bench(() -> fingerprintColumns_defective(columns, ignored));
|
||||
long tFix = bench(() -> fingerprintColumns_fixed(columns, ignored));
|
||||
double ratio = (double) tDef / Math.max(tFix, 1);
|
||||
System.out.printf("PASS cockroach-0002 perf defective=%dms fixed=%dms ratio=%.1fx%n",
|
||||
tDef / 1_000_000, tFix / 1_000_000, ratio);
|
||||
if (ratio >= 1.5) pass++; else { System.out.println("FAIL cockroach-0002 perf ratio too low"); fail++; }
|
||||
}
|
||||
|
||||
// --- cockroach-0003 correctness ---
|
||||
{
|
||||
Set<String> current = new HashSet<>();
|
||||
for (int i = 0; i < 100; i++) current.add("role_" + i);
|
||||
List<String> desired = new ArrayList<>();
|
||||
for (int i = 0; i < 60; i++) desired.add("role_" + i);
|
||||
|
||||
List<String> def = rolesToRevoke_defective(current, desired);
|
||||
List<String> fix = rolesToRevoke_fixed(current, desired);
|
||||
Collections.sort(def); Collections.sort(fix);
|
||||
if (def.equals(fix)) {
|
||||
System.out.println("PASS cockroach-0003 correctness (toRevoke=" + def.size() + ")");
|
||||
pass++;
|
||||
} else {
|
||||
System.out.println("FAIL cockroach-0003 correctness def=" + def + " fix=" + fix);
|
||||
fail++;
|
||||
}
|
||||
}
|
||||
|
||||
// --- cockroach-0003 performance ---
|
||||
{
|
||||
Set<String> current = new HashSet<>();
|
||||
for (int i = 0; i < 2000; i++) current.add("role_" + i);
|
||||
List<String> desired = new ArrayList<>();
|
||||
for (int i = 0; i < 1000; i++) desired.add("role_" + i);
|
||||
|
||||
long tDef = bench(() -> rolesToRevoke_defective(current, desired));
|
||||
long tFix = bench(() -> rolesToRevoke_fixed(current, desired));
|
||||
double ratio = (double) tDef / Math.max(tFix, 1);
|
||||
System.out.printf("PASS cockroach-0003 perf defective=%dms fixed=%dms ratio=%.1fx%n",
|
||||
tDef / 1_000_000, tFix / 1_000_000, ratio);
|
||||
if (ratio >= 2.0) pass++; else { System.out.println("FAIL cockroach-0003 perf ratio too low"); fail++; }
|
||||
}
|
||||
|
||||
System.out.println();
|
||||
System.out.println("Results: " + pass + " PASS, " + fail + " FAIL");
|
||||
if (fail > 0) System.exit(1);
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue