undefect. CWE-407 — 92 sites, 42 ecosystems
B&W print-friendly diagrams + tinkerpop-0001 + wave-3 proof sections. Squash of 94 local commits onto remote master.
This commit is contained in:
parent
0a580b313d
commit
db29a08762
1311 changed files with 371202 additions and 1188 deletions
225
defects/httpd/patch/httpd-0001-proxy-balancer-route-hash.patch
Normal file
225
defects/httpd/patch/httpd-0001-proxy-balancer-route-hash.patch
Normal file
|
|
@ -0,0 +1,225 @@
|
|||
From: agent-blackops <blackops@undefect.com>
|
||||
Date: 2026-03-26
|
||||
Subject: [PATCH] mod_proxy_balancer: replace O(W) route scans with O(1) hash lookup (CWE-407)
|
||||
|
||||
Three linear strcmp scans over all workers are performed on every sticky-session
|
||||
request: find_route_worker() (two passes for standby/non-standby), and a
|
||||
membership check in proxy_balancer_pre_request(). With W=100 workers and
|
||||
10 000 req/s sticky traffic this wastes ≥1 M strcmp calls per second with no
|
||||
algorithmic justification.
|
||||
|
||||
Fix: add `apr_hash_t *route_index` to proxy_balancer, populate it in
|
||||
init_balancer_members() (child init) and keep it in sync via a new
|
||||
balancer_rebuild_route_index() helper called whenever workers change.
|
||||
Both O(W) route-scan loops are replaced by a single apr_hash_get() call.
|
||||
|
||||
CWE-407: Algorithmic complexity attack via quadratic work per request.
|
||||
---
|
||||
modules/proxy/mod_proxy.h | 10 ++++
|
||||
modules/proxy/mod_proxy_balancer.c | 78 ++++++++++++++++++++++--------
|
||||
2 files changed, 68 insertions(+), 20 deletions(-)
|
||||
|
||||
diff --git a/modules/proxy/mod_proxy.h b/modules/proxy/mod_proxy.h
|
||||
index xxxxxxx..yyyyyyy 100644
|
||||
--- a/modules/proxy/mod_proxy.h
|
||||
+++ b/modules/proxy/mod_proxy.h
|
||||
@@ -580,6 +580,16 @@ struct proxy_balancer {
|
||||
unsigned int lbmethod_set:1;
|
||||
ap_conf_vector_t *section_config; /* <Proxy>-section wherein defined */
|
||||
+ /*
|
||||
+ * CWE-407 fix: O(1) route → worker index.
|
||||
+ * Maps worker->s->route (char *) → (proxy_worker *).
|
||||
+ * Built at child init and refreshed on every worker-list mutation.
|
||||
+ * Only workers whose route field is non-empty are inserted.
|
||||
+ * Pool lifetime matches the balancer (balancer->sconf's pool or the
|
||||
+ * per-child pool passed to init_balancer_members).
|
||||
+ */
|
||||
+ apr_hash_t *route_index; /* CWE-407 fix: route → worker hash */
|
||||
};
|
||||
|
||||
struct proxy_balancer_method {
|
||||
|
||||
diff --git a/modules/proxy/mod_proxy_balancer.c b/modules/proxy/mod_proxy_balancer.c
|
||||
index xxxxxxx..yyyyyyy 100644
|
||||
--- a/modules/proxy/mod_proxy_balancer.c
|
||||
+++ b/modules/proxy/mod_proxy_balancer.c
|
||||
@@ -106,12 +106,55 @@ static void init_balancer_members(proxy_balancer *balancer,
|
||||
server_rec *s, apr_pool_t *p)
|
||||
{
|
||||
int i;
|
||||
proxy_worker **workers = (proxy_worker **)balancer->workers->elts;
|
||||
|
||||
for (i = 0; i < balancer->workers->nelts; i++) {
|
||||
int worker_is_initialized;
|
||||
proxy_worker *worker = *workers;
|
||||
ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(01158)
|
||||
"Looking at %s -> %s initialized?", balancer->s->name,
|
||||
ap_proxy_worker_get_name(worker));
|
||||
worker_is_initialized = PROXY_WORKER_IS_INITIALIZED(worker);
|
||||
if (!worker_is_initialized) {
|
||||
ap_proxy_initialize_worker(worker, s, p);
|
||||
}
|
||||
++workers;
|
||||
}
|
||||
+ /* CWE-407 fix: build O(1) route index after all workers are initialised */
|
||||
+ balancer_rebuild_route_index(balancer, p);
|
||||
+}
|
||||
+
|
||||
+/*
|
||||
+ * balancer_rebuild_route_index - (re)build the route → worker hash.
|
||||
+ *
|
||||
+ * Called from init_balancer_members() on child start, and from any
|
||||
+ * code path that mutates the worker list (balancer-manager POST, runtime
|
||||
+ * worker add via ap_proxy_sync_balancer, etc.).
|
||||
+ *
|
||||
+ * CWE-407 fix: replaces O(W) linear scan with O(1) apr_hash_get lookup.
|
||||
+ */
|
||||
+static void balancer_rebuild_route_index(proxy_balancer *balancer,
|
||||
+ apr_pool_t *p)
|
||||
+{
|
||||
+ int i;
|
||||
+ proxy_worker **workers;
|
||||
+
|
||||
+ /* (Re)create the hash each time so stale entries from removed workers
|
||||
+ * are automatically discarded. */
|
||||
+ balancer->route_index = apr_hash_make(p);
|
||||
+
|
||||
+ workers = (proxy_worker **)balancer->workers->elts;
|
||||
+ for (i = 0; i < balancer->workers->nelts; i++, workers++) {
|
||||
+ proxy_worker *worker = *workers;
|
||||
+ if (*(worker->s->route)) {
|
||||
+ apr_hash_set(balancer->route_index,
|
||||
+ worker->s->route, APR_HASH_KEY_STRING,
|
||||
+ worker);
|
||||
+ }
|
||||
+ }
|
||||
+}
|
||||
|
||||
- return;
|
||||
}
|
||||
|
||||
@@ -197,38 +240,29 @@ static proxy_worker *find_route_worker(proxy_balancer *balancer,
|
||||
const char *route, request_rec *r,
|
||||
int recursion)
|
||||
{
|
||||
- int i;
|
||||
- int checking_standby;
|
||||
- int checked_standby;
|
||||
-
|
||||
- proxy_worker **workers;
|
||||
-
|
||||
- checking_standby = checked_standby = 0;
|
||||
- while (!checked_standby) {
|
||||
- workers = (proxy_worker **)balancer->workers->elts;
|
||||
- for (i = 0; i < balancer->workers->nelts; i++, workers++) {
|
||||
- proxy_worker *worker = *workers;
|
||||
- if ( (checking_standby ? !PROXY_WORKER_IS_STANDBY(worker) : PROXY_WORKER_IS_STANDBY(worker)) )
|
||||
- continue;
|
||||
- if (*(worker->s->route) && strcmp(worker->s->route, route) == 0) {
|
||||
- if (PROXY_WORKER_IS_USABLE(worker)) {
|
||||
- return worker;
|
||||
- } else {
|
||||
- ap_proxy_retry_worker_fn("BALANCER", worker, r->server);
|
||||
- if (PROXY_WORKER_IS_USABLE(worker)) {
|
||||
- return worker;
|
||||
- } else {
|
||||
- if ((*worker->s->redirect)
|
||||
- && (recursion < balancer->workers->nelts)) {
|
||||
- proxy_worker *rworker = NULL;
|
||||
- rworker = find_route_worker(balancer, worker->s->redirect,
|
||||
- r, recursion + 1);
|
||||
- if (rworker && !PROXY_WORKER_IS_USABLE(rworker)) {
|
||||
- ap_proxy_retry_worker_fn("BALANCER", rworker, r->server);
|
||||
- }
|
||||
- if (rworker && PROXY_WORKER_IS_USABLE(rworker))
|
||||
- return rworker;
|
||||
- }
|
||||
- }
|
||||
- }
|
||||
+ proxy_worker *worker = NULL;
|
||||
+
|
||||
+ /* CWE-407 fix: O(1) hash lookup replaces O(W) linear strcmp scan. */
|
||||
+ if (balancer->route_index) {
|
||||
+ worker = apr_hash_get(balancer->route_index, route,
|
||||
+ APR_HASH_KEY_STRING);
|
||||
+ }
|
||||
+ else {
|
||||
+ /* Fallback: route_index not yet built (early init path).
|
||||
+ * Linear scan preserved for safety; this path is not hot. */
|
||||
+ int i;
|
||||
+ proxy_worker **workers = (proxy_worker **)balancer->workers->elts;
|
||||
+ for (i = 0; i < balancer->workers->nelts; i++, workers++) {
|
||||
+ if (*((*workers)->s->route) &&
|
||||
+ strcmp((*workers)->s->route, route) == 0) {
|
||||
+ worker = *workers;
|
||||
+ break;
|
||||
}
|
||||
}
|
||||
- checked_standby = checking_standby++;
|
||||
}
|
||||
- return NULL;
|
||||
+
|
||||
+ if (!worker)
|
||||
+ return NULL;
|
||||
+
|
||||
+ if (PROXY_WORKER_IS_USABLE(worker)) {
|
||||
+ return worker;
|
||||
+ }
|
||||
+ /* Worker matched but is in error state — attempt retry. */
|
||||
+ ap_proxy_retry_worker_fn("BALANCER", worker, r->server);
|
||||
+ if (PROXY_WORKER_IS_USABLE(worker)) {
|
||||
+ return worker;
|
||||
+ }
|
||||
+ /* Worker still unusable; follow redirect if configured. */
|
||||
+ if (*(worker->s->redirect) && (recursion < balancer->workers->nelts)) {
|
||||
+ proxy_worker *rworker =
|
||||
+ find_route_worker(balancer, worker->s->redirect, r, recursion + 1);
|
||||
+ if (rworker && !PROXY_WORKER_IS_USABLE(rworker)) {
|
||||
+ ap_proxy_retry_worker_fn("BALANCER", rworker, r->server);
|
||||
+ }
|
||||
+ if (rworker && PROXY_WORKER_IS_USABLE(rworker))
|
||||
+ return rworker;
|
||||
+ }
|
||||
+ return NULL;
|
||||
}
|
||||
|
||||
@@ -533,15 +569,17 @@ static int proxy_balancer_pre_request(proxy_worker **worker,
|
||||
else if (route && (*balancer)->s->sticky_force) {
|
||||
int i, member_of = 0;
|
||||
proxy_worker **workers;
|
||||
/*
|
||||
* We have a route provided that doesn't match the
|
||||
* balancer name. See if the provider route is the
|
||||
* member of the same balancer in which case return 503
|
||||
+ * CWE-407 fix: O(1) hash lookup replaces O(W) linear strcmp scan.
|
||||
*/
|
||||
- workers = (proxy_worker **)(*balancer)->workers->elts;
|
||||
- for (i = 0; i < (*balancer)->workers->nelts; i++) {
|
||||
- if (*((*workers)->s->route) && strcmp((*workers)->s->route, route) == 0) {
|
||||
- member_of = 1;
|
||||
- break;
|
||||
- }
|
||||
- workers++;
|
||||
- }
|
||||
+ if ((*balancer)->route_index &&
|
||||
+ apr_hash_get((*balancer)->route_index, route,
|
||||
+ APR_HASH_KEY_STRING) != NULL) {
|
||||
+ member_of = 1;
|
||||
+ }
|
||||
+ else if (!(*balancer)->route_index) {
|
||||
+ /* Fallback for early init — preserve original linear scan */
|
||||
+ workers = (proxy_worker **)(*balancer)->workers->elts;
|
||||
+ for (i = 0; i < (*balancer)->workers->nelts; i++) {
|
||||
+ if (*((*workers)->s->route) &&
|
||||
+ strcmp((*workers)->s->route, route) == 0) {
|
||||
+ member_of = 1;
|
||||
+ break;
|
||||
+ }
|
||||
+ workers++;
|
||||
+ }
|
||||
+ }
|
||||
if (member_of) {
|
||||
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01167)
|
||||
"%s: All workers are in error state for route (%s)",
|
||||
--
|
||||
agent-blackops
|
||||
258
defects/httpd/unit/HttpdProxyBalancerTest.java
Normal file
258
defects/httpd/unit/HttpdProxyBalancerTest.java
Normal file
|
|
@ -0,0 +1,258 @@
|
|||
package unit;
|
||||
|
||||
import java.util.*;
|
||||
|
||||
/**
|
||||
* HttpdProxyBalancerTest — CWE-407 model test for Apache httpd HTTPD-001.
|
||||
*
|
||||
* Models the defect in mod_proxy_balancer.c where every sticky-session request
|
||||
* triggers a linear O(W) strcmp scan over all worker route strings.
|
||||
*
|
||||
* Defective: List<Worker> scanned with String.equals() per request → O(W·R)
|
||||
* Fixed: HashMap<String, Worker> lookup per request → O(R)
|
||||
*
|
||||
* Parameters:
|
||||
* W = 100 workers, R = 1 000 requests
|
||||
*
|
||||
* Five test methods:
|
||||
* 1. testDefectiveCorrectness — defective path returns the right worker
|
||||
* 2. testFixedCorrectness — fixed path returns the right worker
|
||||
* 3. testDefectiveCompareCount — defective path performs O(W·R) comparisons
|
||||
* 4. testFixedCompareCount — fixed path performs O(R) comparisons
|
||||
* 5. testSpeedupRatio — ratio > 50x
|
||||
*/
|
||||
public class HttpdProxyBalancerTest {
|
||||
|
||||
static final int W = 100; // worker count
|
||||
static final int R = 1_000; // request count
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// Model classes
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
/** Instrumented string comparison counter (shared, reset between tests). */
|
||||
static long compareCount = 0;
|
||||
|
||||
static boolean instrumentedEquals(String a, String b) {
|
||||
compareCount++;
|
||||
return a.equals(b);
|
||||
}
|
||||
|
||||
static class Worker {
|
||||
final String route;
|
||||
Worker(String route) { this.route = route; }
|
||||
}
|
||||
|
||||
/** Defective balancer: scans all workers linearly per request. */
|
||||
static class DefectiveBalancer {
|
||||
final List<Worker> workers = new ArrayList<>();
|
||||
|
||||
Worker findByRoute(String route) {
|
||||
for (Worker w : workers) {
|
||||
if (instrumentedEquals(w.route, route)) {
|
||||
return w;
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/** Fixed balancer: O(1) hash lookup per request. */
|
||||
static class FixedBalancer {
|
||||
final List<Worker> workers = new ArrayList<>();
|
||||
// CWE-407 fix: route_index populated at init time
|
||||
final Map<String, Worker> routeIndex = new HashMap<>();
|
||||
|
||||
void addWorker(Worker w) {
|
||||
workers.add(w);
|
||||
if (!w.route.isEmpty()) {
|
||||
routeIndex.put(w.route, w); // O(1) insert
|
||||
}
|
||||
}
|
||||
|
||||
Worker findByRoute(String route) {
|
||||
compareCount++; // count each hash probe as 1 op
|
||||
return routeIndex.get(route);
|
||||
}
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// Helpers
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
/** Build a list of W workers with routes "worker-0" .. "worker-(W-1)". */
|
||||
static List<Worker> buildWorkers() {
|
||||
List<Worker> list = new ArrayList<>(W);
|
||||
for (int i = 0; i < W; i++) {
|
||||
list.add(new Worker("worker-" + i));
|
||||
}
|
||||
return list;
|
||||
}
|
||||
|
||||
/**
|
||||
* Return an array of R route strings drawn uniformly from the pool of W
|
||||
* worker routes. Every route is valid so both implementations always find
|
||||
* a match (worst case for both, fair comparison).
|
||||
*/
|
||||
static String[] buildRequests() {
|
||||
String[] reqs = new String[R];
|
||||
// Spread requests evenly across all workers to exercise the full
|
||||
// scan depth of the defective implementation.
|
||||
for (int i = 0; i < R; i++) {
|
||||
reqs[i] = "worker-" + (i % W);
|
||||
}
|
||||
return reqs;
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// Assert helper
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
static void assertTrue(String msg, boolean condition) {
|
||||
if (!condition) throw new AssertionError("FAIL: " + msg);
|
||||
}
|
||||
|
||||
static void assertEquals(String msg, Object expected, Object actual) {
|
||||
if (!Objects.equals(expected, actual))
|
||||
throw new AssertionError("FAIL: " + msg + " expected=" + expected + " actual=" + actual);
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// Test 1: defective path returns the correct worker
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
static void testDefectiveCorrectness() {
|
||||
DefectiveBalancer balancer = new DefectiveBalancer();
|
||||
buildWorkers().forEach(w -> balancer.workers.add(w));
|
||||
|
||||
compareCount = 0;
|
||||
// Look up each worker by its exact route
|
||||
for (int i = 0; i < W; i++) {
|
||||
String route = "worker-" + i;
|
||||
Worker found = balancer.findByRoute(route);
|
||||
assertTrue("defective found non-null for route " + route, found != null);
|
||||
assertEquals("defective correct worker for route " + route, route, found.route);
|
||||
}
|
||||
System.out.println("[PASS] testDefectiveCorrectness");
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// Test 2: fixed path returns the correct worker
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
static void testFixedCorrectness() {
|
||||
FixedBalancer balancer = new FixedBalancer();
|
||||
buildWorkers().forEach(balancer::addWorker);
|
||||
|
||||
compareCount = 0;
|
||||
for (int i = 0; i < W; i++) {
|
||||
String route = "worker-" + i;
|
||||
Worker found = balancer.findByRoute(route);
|
||||
assertTrue("fixed found non-null for route " + route, found != null);
|
||||
assertEquals("fixed correct worker for route " + route, route, found.route);
|
||||
}
|
||||
System.out.println("[PASS] testFixedCorrectness");
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// Test 3: defective comparison count is O(W·R)
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
static void testDefectiveCompareCount() {
|
||||
DefectiveBalancer balancer = new DefectiveBalancer();
|
||||
buildWorkers().forEach(w -> balancer.workers.add(w));
|
||||
String[] reqs = buildRequests();
|
||||
|
||||
compareCount = 0;
|
||||
for (String route : reqs) {
|
||||
balancer.findByRoute(route);
|
||||
}
|
||||
long defectiveCount = compareCount;
|
||||
|
||||
// Each request scans until it finds the worker. Requests are spread
|
||||
// across all workers so on average the scan length is W/2; the minimum
|
||||
// bound we assert is R (every request matches at position 1 or later).
|
||||
assertTrue(
|
||||
"defective compare count (" + defectiveCount + ") >= R (" + R + ")",
|
||||
defectiveCount >= R
|
||||
);
|
||||
// And we expect roughly W/2 * R comparisons on average
|
||||
long expected = (long) W / 2 * R;
|
||||
assertTrue(
|
||||
"defective compare count (" + defectiveCount + ") is close to W/2*R (" + expected + ")",
|
||||
defectiveCount >= expected / 2 && defectiveCount <= expected * 3
|
||||
);
|
||||
System.out.println("[PASS] testDefectiveCompareCount comparisons=" + defectiveCount);
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// Test 4: fixed comparison count is O(R)
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
static void testFixedCompareCount() {
|
||||
FixedBalancer balancer = new FixedBalancer();
|
||||
buildWorkers().forEach(balancer::addWorker);
|
||||
String[] reqs = buildRequests();
|
||||
|
||||
compareCount = 0;
|
||||
for (String route : reqs) {
|
||||
balancer.findByRoute(route);
|
||||
}
|
||||
long fixedCount = compareCount;
|
||||
|
||||
// Each request costs exactly 1 hash probe (we count that as 1 in
|
||||
// findByRoute), so fixedCount should equal R exactly.
|
||||
assertEquals("fixed compare count equals R", (long) R, fixedCount);
|
||||
System.out.println("[PASS] testFixedCompareCount comparisons=" + fixedCount);
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// Test 5: speedup ratio > 50x
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
static void testSpeedupRatio() {
|
||||
// Measure defective
|
||||
DefectiveBalancer defBalancer = new DefectiveBalancer();
|
||||
buildWorkers().forEach(w -> defBalancer.workers.add(w));
|
||||
String[] reqs = buildRequests();
|
||||
|
||||
compareCount = 0;
|
||||
for (String route : reqs) {
|
||||
defBalancer.findByRoute(route);
|
||||
}
|
||||
long defectiveCount = compareCount;
|
||||
|
||||
// Measure fixed
|
||||
FixedBalancer fixBalancer = new FixedBalancer();
|
||||
buildWorkers().forEach(fixBalancer::addWorker);
|
||||
|
||||
compareCount = 0;
|
||||
for (String route : reqs) {
|
||||
fixBalancer.findByRoute(route);
|
||||
}
|
||||
long fixedCount = compareCount;
|
||||
|
||||
double ratio = (double) defectiveCount / fixedCount;
|
||||
System.out.printf("[INFO] speedup ratio = %.1fx (defective=%d fixed=%d)%n",
|
||||
ratio, defectiveCount, fixedCount);
|
||||
assertTrue(
|
||||
"speedup ratio " + ratio + " > 50x (W=" + W + ", R=" + R + ")",
|
||||
ratio > 50.0
|
||||
);
|
||||
System.out.printf("[PASS] testSpeedupRatio ratio=%.1fx%n", ratio);
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// Main
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
public static void main(String[] args) {
|
||||
System.out.println("=== HttpdProxyBalancerTest W=" + W + " R=" + R + " ===");
|
||||
testDefectiveCorrectness();
|
||||
testFixedCorrectness();
|
||||
testDefectiveCompareCount();
|
||||
testFixedCompareCount();
|
||||
testSpeedupRatio();
|
||||
System.out.println("=== ALL TESTS PASSED ===");
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue