nmap-0002 + haproxy-0004 + nginx-0004 + weechat-0003 + zeek-0002 + curl-0004: 6 new CWE-407 defects in network tools; count 693→699
nmap-0002: nmap.cc merge_port_lists O(N²) port dedup → unordered_set O(N); ~65000x at max range haproxy-0004: http_ana.c http_capture_headers O(H×C) cap_hdr walk per request → pre-built HashMap O(H) nginx-0004: ngx_http_upstream_keepalive_module.c keepalive_get_peer O(C) sockaddr scan per upstream request → HashMap O(1) weechat-0003: irc-channel.c irc_channel_search O(C) linked-list scan per message handler → channels_hashtable O(1) zeek-0002: Attr.cc Attributes::AddAttrs O(A²) triple-Find/RemoveAttr per attr → unordered_map index O(A) curl-0004: mime.c search_header O(P×H) 3x per part per mime_add_headers → pre-indexed header name set O(P)
This commit is contained in:
parent
421f3352c7
commit
ba818693db
21 changed files with 2213 additions and 0 deletions
|
|
@ -0,0 +1,93 @@
|
|||
# nginx-0004 — ngx_http_upstream_keepalive: O(C) linear cache scan per upstream request
|
||||
|
||||
## Ecosystem
|
||||
nginx (C)
|
||||
|
||||
## Severity
|
||||
MEDIUM — hot path: executed on every upstream request that can reuse a
|
||||
keepalive connection; scales with `keepalive` directive value
|
||||
|
||||
## Location
|
||||
`src/http/modules/ngx_http_upstream_keepalive_module.c`
|
||||
- Function: `ngx_http_upstream_keepalive_get_peer` (~line 212)
|
||||
- Inner loop: `for (q = ngx_queue_head(cache); q != ngx_queue_sentinel(cache); ...)` (~line 229)
|
||||
|
||||
## Description
|
||||
|
||||
When an upstream request needs a connection, `ngx_http_upstream_keepalive_get_peer`
|
||||
scans the entire keepalive cache queue to find a cached connection matching
|
||||
the upstream's `sockaddr`:
|
||||
|
||||
```c
|
||||
/* search cache for suitable connection */
|
||||
cache = &kp->conf->cache;
|
||||
|
||||
for (q = ngx_queue_head(cache);
|
||||
q != ngx_queue_sentinel(cache);
|
||||
q = ngx_queue_next(q)) // O(C) scan
|
||||
{
|
||||
item = ngx_queue_data(q, ngx_http_upstream_keepalive_cache_t, queue);
|
||||
c = item->connection;
|
||||
|
||||
if (ngx_memn2cmp((u_char *) &item->sockaddr, (u_char *) pc->sockaddr,
|
||||
item->socklen, pc->socklen) == 0)
|
||||
{
|
||||
ngx_queue_remove(q);
|
||||
goto found;
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
With `keepalive N` set to a large value (e.g., `keepalive 1000` or `keepalive 10000`
|
||||
for backends with many servers), this scan processes up to N entries on every upstream
|
||||
request. Under high throughput, this O(C) scan becomes a bottleneck:
|
||||
|
||||
- 10,000 RPS × C=1000 keepalive cache entries = 10 million comparisons/second
|
||||
just for connection reuse lookups
|
||||
|
||||
## Fix
|
||||
|
||||
Index the keepalive cache by sockaddr using a hash table keyed on the
|
||||
(family, addr, port) tuple, giving O(1) lookup per upstream request:
|
||||
|
||||
```c
|
||||
--- a/src/http/modules/ngx_http_upstream_keepalive_module.c
|
||||
+++ b/src/http/modules/ngx_http_upstream_keepalive_module.c
|
||||
@@ struct ngx_http_upstream_keepalive_srv_conf_s {
|
||||
ngx_queue_t cache; /* LRU queue of cached connections */
|
||||
ngx_queue_t free; /* free items */
|
||||
+ ngx_hash_t cache_hash; /* sockaddr → cached items list */
|
||||
|
||||
- /* search cache for suitable connection */
|
||||
- for (q = ngx_queue_head(cache);
|
||||
- q != ngx_queue_sentinel(cache);
|
||||
- q = ngx_queue_next(q))
|
||||
- {
|
||||
- item = ngx_queue_data(q, ...);
|
||||
- if (ngx_memn2cmp(&item->sockaddr, pc->sockaddr, ...) == 0)
|
||||
- goto found;
|
||||
- }
|
||||
+ /* O(1) hash lookup by sockaddr */
|
||||
+ ngx_http_upstream_keepalive_cache_t *item =
|
||||
+ ngx_hash_find(&kp->conf->cache_hash,
|
||||
+ ngx_crc32_long(pc->sockaddr->sa_data, pc->socklen),
|
||||
+ pc->sockaddr, pc->socklen);
|
||||
+ if (item) goto found;
|
||||
```
|
||||
|
||||
## Complexity
|
||||
|
||||
| Variant | Cost per upstream connection attempt |
|
||||
|---------|-------------------------------------|
|
||||
| Before | O(C) — full scan of keepalive cache |
|
||||
| After | O(1) — hash map lookup |
|
||||
| Speedup | C× (keepalive pool size) |
|
||||
|
||||
## Notes
|
||||
- Default `keepalive` is typically 32-100 for simple setups; some deployments
|
||||
use 1000+ for microservice backends with many upstream servers
|
||||
- nginx's existing LRU queue serves eviction; the hash provides fast lookup
|
||||
- Multiple cached connections to the same server are valid; the hash can
|
||||
return a head-of-list and then remove from the LRU queue
|
||||
- `ngx_queue_insert_head(&kp->conf->free, q)` after successful lookup
|
||||
must also update the hash
|
||||
90
defects/nginx/unit/NginxKeepaliveCacheLinearScanTest.java
Normal file
90
defects/nginx/unit/NginxKeepaliveCacheLinearScanTest.java
Normal file
|
|
@ -0,0 +1,90 @@
|
|||
package unit;
|
||||
|
||||
import java.util.*;
|
||||
|
||||
/**
|
||||
* nginx-0004 — CWE-407: ngx_http_upstream_keepalive_get_peer O(C) per upstream request
|
||||
*
|
||||
* Models src/http/modules/ngx_http_upstream_keepalive_module.c
|
||||
* ngx_http_upstream_keepalive_get_peer() (~line 212):
|
||||
* Slow: iterates free[] queue scanning for matching sockaddr → O(C) per request
|
||||
* where C = keepalive cache size (default 0, but configured 100–10000 in production)
|
||||
* Fast: HashMap<sockaddrKey, connection> → O(1) per request
|
||||
*
|
||||
* Hot path: called on every upstream HTTP request that could reuse a keepalive connection.
|
||||
* With `keepalive 1000` configured and many distinct upstreams, degrades to O(1000) per request.
|
||||
*/
|
||||
public class NginxKeepaliveCacheLinearScanTest {
|
||||
|
||||
// Simulates ngx_connection_t / ngx_peer_connection_t with a sockaddr key
|
||||
static class KeepaliveConn {
|
||||
final String sockaddrKey; // e.g., "192.168.1.1:8080"
|
||||
KeepaliveConn(String sockaddrKey) { this.sockaddrKey = sockaddrKey; }
|
||||
}
|
||||
|
||||
// --- SLOW: O(C) linear scan (defect) ---
|
||||
// Models: for (q = ngx_queue_head(cache); q != ngx_queue_sentinel(cache); q = ngx_queue_next(q))
|
||||
// item = ngx_queue_data(q, ...); if (ngx_memn2cmp(sockaddr, item->sockaddr) == 0) found
|
||||
static long keepaliveGetPeerSlow(List<KeepaliveConn> cache, String target) {
|
||||
long ops = 0;
|
||||
for (KeepaliveConn conn : cache) {
|
||||
ops++;
|
||||
if (conn.sockaddrKey.equals(target)) break;
|
||||
}
|
||||
return ops;
|
||||
}
|
||||
|
||||
// --- FAST: O(1) hash lookup (fix) ---
|
||||
// Models: pre-built HashMap keyed by sockaddr at connection cache time
|
||||
static long keepaliveGetPeerFast(Map<String, KeepaliveConn> index, String target) {
|
||||
index.get(target);
|
||||
return 1;
|
||||
}
|
||||
|
||||
// Build a keepalive cache with C connections to distinct upstream addresses
|
||||
static List<KeepaliveConn> buildCache(int C) {
|
||||
List<KeepaliveConn> cache = new ArrayList<>(C);
|
||||
for (int i = 0; i < C; i++)
|
||||
cache.add(new KeepaliveConn("10.0." + (i / 256) + "." + (i % 256) + ":8080"));
|
||||
return cache;
|
||||
}
|
||||
|
||||
static Map<String, KeepaliveConn> buildIndex(List<KeepaliveConn> cache) {
|
||||
Map<String, KeepaliveConn> map = new HashMap<>(cache.size() * 2);
|
||||
for (KeepaliveConn c : cache) map.put(c.sockaddrKey, c);
|
||||
return map;
|
||||
}
|
||||
|
||||
public static void main(String[] args) {
|
||||
System.out.println("nginx-0004 CWE-407: keepalive_get_peer O(C) vs O(1)");
|
||||
System.out.println("======================================================");
|
||||
|
||||
// R = number of upstream requests, C = keepalive cache size
|
||||
// target is the last entry (worst-case scan)
|
||||
int[][] params = { {100, 100}, {500, 500}, {1000, 1000} };
|
||||
|
||||
for (int[] p : params) {
|
||||
int R = p[0], C = p[1];
|
||||
List<KeepaliveConn> cache = buildCache(C);
|
||||
Map<String, KeepaliveConn> index = buildIndex(cache);
|
||||
|
||||
// Worst-case: target is the last connection in the cache
|
||||
String target = cache.get(C - 1).sockaddrKey;
|
||||
|
||||
long slowTotal = 0, fastTotal = 0;
|
||||
for (int r = 0; r < R; r++) {
|
||||
slowTotal += keepaliveGetPeerSlow(cache, target);
|
||||
fastTotal += keepaliveGetPeerFast(index, target);
|
||||
}
|
||||
|
||||
double ratio = (double) slowTotal / fastTotal;
|
||||
System.out.printf(" C=%4d cache, R=%4d requests (worst-case): slow=%,8d ops fast=%,5d ops speedup=%.0fx%n",
|
||||
C, R, slowTotal, fastTotal, ratio);
|
||||
|
||||
assert ratio >= (double) C / 2 :
|
||||
"Expected speedup >= " + (C/2) + "x but got " + ratio + " (C=" + C + ")";
|
||||
}
|
||||
|
||||
System.out.println("\nPASS");
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue