ninja-0001/ninja-0002: CWE-407 depfile dedup and output validation O(N²)→O(N)
ninja-0001: depfile_parser.cc std::find over ins_/outs_ vectors is O(N) per token → O(N²) total parse; fix adds unordered_set ins_seen_/outs_seen_ members to DepfileParser for O(1) duplicate detection (490x at N=1000). ninja-0002: graph.cc std::find_if over edge->outputs_ called for each depfile output token is O(M×N); fix builds unordered_set from edge outputs once before the loop for O(M+N) total (125x at M=N=500).
This commit is contained in:
parent
740f17256b
commit
f176e86fbd
4 changed files with 311 additions and 1 deletions
|
|
@ -719,5 +719,8 @@
|
|||
"vlc-0002": "UNDF-2026-000000718",
|
||||
"weechat-0003": "UNDF-2026-000000719",
|
||||
"zeek-0002": "UNDF-2026-000000720",
|
||||
"spidermonkey-0005": "UNDF-2026-000000721"
|
||||
"spidermonkey-0005": "UNDF-2026-000000721",
|
||||
"distlib-0002": "UNDF-2026-000000722",
|
||||
"ninja-0002": "UNDF-2026-000000723",
|
||||
"zookeeper-0002": "UNDF-2026-000000724"
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,70 @@
|
|||
# UNDF: UNDF-2026-000000186
|
||||
--- a/src/depfile_parser.h
|
||||
+++ b/src/depfile_parser.h
|
||||
@@ -15,8 +15,9 @@
|
||||
#ifndef NINJA_DEPFILE_PARSER_H_
|
||||
#define NINJA_DEPFILE_PARSER_H_
|
||||
|
||||
+#include <string>
|
||||
+#include <unordered_set>
|
||||
#include <vector>
|
||||
-#include <string>
|
||||
|
||||
#include "string_piece.h"
|
||||
|
||||
@@ -29,8 +30,13 @@ struct DepfileParser {
|
||||
/// Parse an input file. Input must be NUL-terminated.
|
||||
/// Warning: may mutate the content in-place and parsed StringPieces are
|
||||
/// pointers within it.
|
||||
bool Parse(std::string* content, std::string* err);
|
||||
|
||||
std::vector<StringPiece> outs_;
|
||||
std::vector<StringPiece> ins_;
|
||||
+ /// CWE-407 fix: O(1) duplicate detection via unordered_set instead of
|
||||
+ /// O(N) std::find over ins_/outs_ vectors. Membership is checked here;
|
||||
+ /// ins_/outs_ vectors remain the ordered result for callers.
|
||||
+ std::unordered_set<std::string> ins_seen_;
|
||||
+ std::unordered_set<std::string> outs_seen_;
|
||||
DepfileParserOptions options_;
|
||||
};
|
||||
|
||||
--- a/src/depfile_parser.cc
|
||||
+++ b/src/depfile_parser.cc
|
||||
@@ -15,6 +15,7 @@
|
||||
#include "depfile_parser.h"
|
||||
#include "util.h"
|
||||
|
||||
+#include <unordered_set>
|
||||
#include <algorithm>
|
||||
|
||||
using namespace std;
|
||||
@@ -334,16 +335,19 @@ bool DepfileParser::Parse(string* content, string* err) {
|
||||
if (len > 0) {
|
||||
is_empty = false;
|
||||
StringPiece piece = StringPiece(filename, len);
|
||||
- // If we've seen this as an input before, skip it.
|
||||
- std::vector<StringPiece>::iterator pos = std::find(ins_.begin(), ins_.end(), piece);
|
||||
- if (pos == ins_.end()) {
|
||||
+ // CWE-407 fix: O(1) set lookup replaces O(N) std::find scan.
|
||||
+ // Large depfiles (monorepo builds) with N=10,000+ entries made this
|
||||
+ // O(N²) over the full parse; unordered_set gives O(N) total.
|
||||
+ std::string piece_str(piece.str_, piece.len_);
|
||||
+ if (ins_seen_.find(piece_str) == ins_seen_.end()) {
|
||||
if (is_dependency) {
|
||||
if (poisoned_input) {
|
||||
*err = "inputs may not also have inputs";
|
||||
return false;
|
||||
}
|
||||
// New input.
|
||||
+ ins_seen_.insert(piece_str);
|
||||
ins_.push_back(piece);
|
||||
} else {
|
||||
// Check for a new output.
|
||||
- if (std::find(outs_.begin(), outs_.end(), piece) == outs_.end())
|
||||
+ if (outs_seen_.find(piece_str) == outs_seen_.end()) {
|
||||
+ outs_seen_.insert(piece_str);
|
||||
outs_.push_back(piece);
|
||||
+ }
|
||||
}
|
||||
} else if (!is_dependency) {
|
||||
// We've passed an input on the left side; reject new inputs.
|
||||
42
defects/ninja/patch/ninja-0002-graph-output-set.patch
Normal file
42
defects/ninja/patch/ninja-0002-graph-output-set.patch
Normal file
|
|
@ -0,0 +1,42 @@
|
|||
# UNDF: UNDF-2026-000000723
|
||||
--- a/src/graph.cc
|
||||
+++ b/src/graph.cc
|
||||
@@ -15,6 +15,7 @@
|
||||
#include "graph.h"
|
||||
|
||||
#include <algorithm>
|
||||
+#include <unordered_set>
|
||||
#include <deque>
|
||||
#include <assert.h>
|
||||
#include <stdio.h>
|
||||
@@ -706,13 +706,20 @@ bool ImplicitDepLoader::LoadDepFile(Edge* edge, const string& path,
|
||||
return false;
|
||||
}
|
||||
|
||||
- // Ensure that all mentioned outputs are outputs of the edge.
|
||||
- for (std::vector<StringPiece>::iterator o = depfile.outs_.begin();
|
||||
- o != depfile.outs_.end(); ++o) {
|
||||
- matches m(o);
|
||||
- if (std::find_if(edge->outputs_.begin(), edge->outputs_.end(), m) == edge->outputs_.end()) {
|
||||
- *err = path + ": depfile mentions '" + o->AsString() + "' as an output, but no such output was declared";
|
||||
- return false;
|
||||
- }
|
||||
+ // CWE-407 fix: build an O(1) lookup set from edge outputs once, then
|
||||
+ // validate each depfile output in O(1). The original code was O(M×N)
|
||||
+ // where M = depfile outs and N = edge outputs; with multiple-output edges
|
||||
+ // (e.g. unity builds) both dimensions can be large.
|
||||
+ std::unordered_set<std::string> output_paths;
|
||||
+ output_paths.reserve(edge->outputs_.size());
|
||||
+ for (const Node* out : edge->outputs_) {
|
||||
+ output_paths.insert(out->path());
|
||||
+ }
|
||||
+ // Ensure that all mentioned outputs are outputs of the edge.
|
||||
+ for (std::vector<StringPiece>::iterator o = depfile.outs_.begin();
|
||||
+ o != depfile.outs_.end(); ++o) {
|
||||
+ if (output_paths.find(o->AsString()) == output_paths.end()) {
|
||||
+ *err = path + ": depfile mentions '" + o->AsString() + "' as an output, but no such output was declared";
|
||||
+ return false;
|
||||
+ }
|
||||
}
|
||||
|
||||
return ProcessDepfileDeps(edge, &depfile.ins_, err);
|
||||
195
defects/ninja/unit/NinjaTest.java
Normal file
195
defects/ninja/unit/NinjaTest.java
Normal file
|
|
@ -0,0 +1,195 @@
|
|||
package unit;
|
||||
|
||||
import java.util.*;
|
||||
|
||||
/**
|
||||
* ninja CWE-407 unit tests
|
||||
*
|
||||
* ninja-0001 — depfile_parser.cc: O(N²) duplicate detection via std::find
|
||||
* src/depfile_parser.cc ~line 338: std::find(ins_.begin(), ins_.end(), piece)
|
||||
* and std::find(outs_.begin(), outs_.end(), piece) on every parsed token.
|
||||
* With a depfile of N=10,000 entries each token scan is O(N) → O(N²) total.
|
||||
* Fix: std::unordered_set<std::string> ins_seen_ / outs_seen_ for O(1) lookup.
|
||||
*
|
||||
* ninja-0002 — graph.cc: O(M×N) output validation via std::find_if
|
||||
* src/graph.cc ~line 712: std::find_if(edge->outputs_.begin(), ...) called for
|
||||
* each of M depfile output tokens against N edge outputs.
|
||||
* Fix: build std::unordered_set<std::string> from edge outputs once → O(M+N).
|
||||
*/
|
||||
public class NinjaTest {
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// ninja-0001: depfile duplicate detection
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* Slow path: models std::find(ins_.begin(), ins_.end(), piece).
|
||||
* Returns total comparison operations performed across N tokens.
|
||||
*
|
||||
* For each new token we scan all previously accepted tokens to check for
|
||||
* duplicates → O(k) per token where k is current list size → O(N²) total.
|
||||
*/
|
||||
static long depfileDedupSlow(List<String> tokens) {
|
||||
List<String> ins = new ArrayList<>();
|
||||
long ops = 0;
|
||||
for (String token : tokens) {
|
||||
// models: std::find(ins_.begin(), ins_.end(), piece)
|
||||
for (String existing : ins) {
|
||||
ops++;
|
||||
if (existing.equals(token)) break;
|
||||
}
|
||||
if (!ins.contains(token)) {
|
||||
ins.add(token);
|
||||
}
|
||||
}
|
||||
return ops;
|
||||
}
|
||||
|
||||
/**
|
||||
* Fast path: models unordered_set<string> ins_seen_.find(piece_str).
|
||||
* Returns total operations (each set operation counted as 1).
|
||||
*/
|
||||
static long depfileDedupFast(List<String> tokens) {
|
||||
Set<String> insSeen = new HashSet<>();
|
||||
List<String> ins = new ArrayList<>();
|
||||
long ops = 0;
|
||||
for (String token : tokens) {
|
||||
ops++; // O(1) hash lookup
|
||||
if (!insSeen.contains(token)) {
|
||||
insSeen.add(token);
|
||||
ins.add(token);
|
||||
}
|
||||
}
|
||||
return ops;
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// ninja-0002: depfile output validation
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* Slow path: models std::find_if(edge->outputs_.begin(), edge->outputs_.end(), m)
|
||||
* called for each of M depfile output tokens against N edge outputs.
|
||||
* O(M×N) total.
|
||||
*/
|
||||
static long outputValidationSlow(List<String> depfileOuts, List<String> edgeOutputs) {
|
||||
long ops = 0;
|
||||
for (String depOut : depfileOuts) {
|
||||
// models: std::find_if scanning all edge outputs
|
||||
for (String edgeOut : edgeOutputs) {
|
||||
ops++;
|
||||
if (edgeOut.equals(depOut)) break;
|
||||
}
|
||||
}
|
||||
return ops;
|
||||
}
|
||||
|
||||
/**
|
||||
* Fast path: build unordered_set from edge outputs once, then O(1) per
|
||||
* depfile output token. O(M+N) total.
|
||||
*/
|
||||
static long outputValidationFast(List<String> depfileOuts, List<String> edgeOutputs) {
|
||||
long ops = 0;
|
||||
// Build set: O(N)
|
||||
Set<String> outputPaths = new HashSet<>(edgeOutputs.size() * 2);
|
||||
for (String o : edgeOutputs) {
|
||||
outputPaths.add(o);
|
||||
ops++;
|
||||
}
|
||||
// Validate: O(M)
|
||||
for (String depOut : depfileOuts) {
|
||||
ops++; // O(1) hash lookup
|
||||
outputPaths.contains(depOut);
|
||||
}
|
||||
return ops;
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// Helpers
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
/** Build N unique dependency tokens like "obj/foo_0000.o.d". */
|
||||
static List<String> buildDepTokens(int n, int duplicatePct) {
|
||||
List<String> tokens = new ArrayList<>(n);
|
||||
int uniqueCount = n * (100 - duplicatePct) / 100;
|
||||
if (uniqueCount < 1) uniqueCount = 1;
|
||||
for (int i = 0; i < n; i++) {
|
||||
// Every (100/duplicatePct)-th token is a duplicate of a previous one
|
||||
int idx = (duplicatePct > 0 && i % (100 / duplicatePct) == 0 && i > 0)
|
||||
? (i % uniqueCount)
|
||||
: i;
|
||||
tokens.add("obj/source_" + String.format("%06d", idx) + ".o");
|
||||
}
|
||||
return tokens;
|
||||
}
|
||||
|
||||
/** Build M depfile output tokens all present in edge outputs. */
|
||||
static List<String> buildOutputTokens(int m) {
|
||||
List<String> outs = new ArrayList<>(m);
|
||||
for (int i = 0; i < m; i++)
|
||||
outs.add("out/target_" + String.format("%04d", i) + ".o");
|
||||
return outs;
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// main
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
public static void main(String[] args) {
|
||||
boolean allPass = true;
|
||||
|
||||
// --- ninja-0001: depfile dedup ---
|
||||
System.out.println("ninja-0001 CWE-407: depfile_parser.cc duplicate detection O(N²) vs O(N)");
|
||||
System.out.println("=========================================================================");
|
||||
System.out.println(" N=tokens, 10% duplicates; ops = comparison operations counted");
|
||||
System.out.printf(" %-8s %-14s %-12s %s%n", "N", "slow(O(N²))", "fast(O(N))", "speedup");
|
||||
|
||||
int[] nValues = { 100, 500, 1000 };
|
||||
for (int n : nValues) {
|
||||
List<String> tokens = buildDepTokens(n, 10);
|
||||
long slow = depfileDedupSlow(tokens);
|
||||
long fast = depfileDedupFast(tokens);
|
||||
double ratio = (double) slow / fast;
|
||||
System.out.printf(" %-8d %-14d %-12d %.1fx%n", n, slow, fast, ratio);
|
||||
|
||||
// At N=1000, 10% duplicates: slow should be roughly O(N²/2) >> O(N)
|
||||
// Require at least 10x speedup at N=1000
|
||||
if (n == 1000 && ratio < 10.0) {
|
||||
System.out.println(" FAIL: expected speedup >= 10x at N=1000, got " + ratio);
|
||||
allPass = false;
|
||||
}
|
||||
}
|
||||
|
||||
// --- ninja-0002: output validation ---
|
||||
System.out.println();
|
||||
System.out.println("ninja-0002 CWE-407: graph.cc output validation O(M×N) vs O(M+N)");
|
||||
System.out.println("=================================================================");
|
||||
System.out.println(" M=depfile outputs, N=edge outputs; ops counted");
|
||||
System.out.printf(" %-6s %-6s %-16s %-12s %s%n", "M", "N", "slow(O(M×N))", "fast(O(M+N))", "speedup");
|
||||
|
||||
int[][] params = { {50, 50}, {200, 200}, {500, 500} };
|
||||
for (int[] p : params) {
|
||||
int m = p[0], n = p[1];
|
||||
List<String> depfileOuts = buildOutputTokens(m);
|
||||
List<String> edgeOutputs = buildOutputTokens(n);
|
||||
long slow = outputValidationSlow(depfileOuts, edgeOutputs);
|
||||
long fast = outputValidationFast(depfileOuts, edgeOutputs);
|
||||
double ratio = (double) slow / fast;
|
||||
System.out.printf(" %-6d %-6d %-16d %-12d %.1fx%n", m, n, slow, fast, ratio);
|
||||
|
||||
// At M=N=500: slow = 500*500 = 250,000 ops; fast = 1000 ops → 250x
|
||||
if (m == 500 && ratio < 50.0) {
|
||||
System.out.println(" FAIL: expected speedup >= 50x at M=N=500, got " + ratio);
|
||||
allPass = false;
|
||||
}
|
||||
}
|
||||
|
||||
System.out.println();
|
||||
if (allPass) {
|
||||
System.out.println("PASS");
|
||||
} else {
|
||||
System.out.println("FAIL");
|
||||
System.exit(1);
|
||||
}
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue