engineering/java-topology
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

bullet3/three.js: CWE-407 findings

Browse source 
bullet3-0001: btGhostObject::addOverlappingObjectInternal uses
findLinearSearch (O(N) pointer scan) on every broadphase pair-update
callback. With N objects overlapping a ghost, each tick is O(N²). Fix:
btHashMap<btHashPtr,int> shadow index → O(1) add/remove. 250x at N=500.

three-0001: NodeBuilder.addNode/addSequentialNode use Array.includes
(O(N)) per node during shader build traversal → O(N²). StackNode.generate
uses nodes.indexOf inside filter → O(N²). Fix: shadow with Set → O(1). 250x
at N=500.
This commit is contained in:
russell@unturf.com 2026-03-30 09:25:29 -04:00
parent 99e1a862bd
commit db92c9428a
4 changed files with 447 additions and 0 deletions
Download patch file Download diff file Expand all files Collapse all files

103
defects/bullet3/patch/bullet3-0001-ghost-object-overlapping-hashmap.patch Normal file
View file

@ -0,0 +1,103 @@
# UNDF: (leave blank)
--- a/src/BulletCollision/CollisionDispatch/btGhostObject.h
+++ b/src/BulletCollision/CollisionDispatch/btGhostObject.h
@@ -19,6 +19,7 @@
#include "btCollisionObject.h"
#include "BulletCollision/BroadphaseCollision/btOverlappingPairCallback.h"
#include "LinearMath/btAlignedAllocator.h"
+#include "LinearMath/btHashMap.h"
#include "BulletCollision/BroadphaseCollision/btOverlappingPairCache.h"
#include "btCollisionWorld.h"
@@ -36,6 +37,12 @@ btGhostObject : public btCollisionObject
protected:
btAlignedObjectArray<btCollisionObject*> m_overlappingObjects;
+ // O(1) membership index: maps btCollisionObject* pointer -> index in m_overlappingObjects.
+ // Eliminates O(N) findLinearSearch on every broadphase add/remove callback,
+ // turning the per-tick pair-update loop from O(N²) to O(N). CWE-407.
+ btHashMap<btHashPtr, int> m_overlappingObjectsIndex;
+
public:
btGhostObject();
--- a/src/BulletCollision/CollisionDispatch/btGhostObject.cpp
+++ b/src/BulletCollision/CollisionDispatch/btGhostObject.cpp
@@ -32,10 +32,12 @@ void btGhostObject::addOverlappingObjectInternal(btBroadphaseProxy* otherProxy,
{
btCollisionObject* otherObject = (btCollisionObject*)otherProxy->m_clientObject;
btAssert(otherObject);
- ///if this linearSearch becomes too slow (too many overlapping objects) we should add a more appropriate data structure
- int index = m_overlappingObjects.findLinearSearch(otherObject);
- if (index == m_overlappingObjects.size())
- {
- //not found
+ // O(1) hash lookup replaces O(N) findLinearSearch. CWE-407.
+ if (m_overlappingObjectsIndex.find(btHashPtr(otherObject)) == NULL)
+ {
+ int index = m_overlappingObjects.size();
m_overlappingObjects.push_back(otherObject);
+ m_overlappingObjectsIndex.insert(btHashPtr(otherObject), index);
}
}
@@ -44,11 +46,16 @@ void btGhostObject::removeOverlappingObjectInternal(btBroadphaseProxy* otherProx
btCollisionObject* otherObject = (btCollisionObject*)otherProxy->m_clientObject;
btAssert(otherObject);
- int index = m_overlappingObjects.findLinearSearch(otherObject);
- if (index < m_overlappingObjects.size())
+ int* indexPtr = m_overlappingObjectsIndex.find(btHashPtr(otherObject));
+ if (indexPtr != NULL)
{
- m_overlappingObjects[index] = m_overlappingObjects[m_overlappingObjects.size() - 1];
+ int index = *indexPtr;
+ int lastIndex = m_overlappingObjects.size() - 1;
+ if (index != lastIndex)
+ {
+ // Swap with last; update the moved element's index entry.
+ btCollisionObject* movedObject = m_overlappingObjects[lastIndex];
+ m_overlappingObjects[index] = movedObject;
+ m_overlappingObjectsIndex.insert(btHashPtr(movedObject), index);
+ }
m_overlappingObjects.pop_back();
+ m_overlappingObjectsIndex.remove(btHashPtr(otherObject));
}
}
@@ -68,9 +75,12 @@ void btPairCachingGhostObject::addOverlappingObjectInternal(btBroadphaseProxy* o
btCollisionObject* otherObject = (btCollisionObject*)otherProxy->m_clientObject;
btAssert(otherObject);
- int index = m_overlappingObjects.findLinearSearch(otherObject);
- if (index == m_overlappingObjects.size())
- {
+ if (m_overlappingObjectsIndex.find(btHashPtr(otherObject)) == NULL)
+ {
+ int index = m_overlappingObjects.size();
m_overlappingObjects.push_back(otherObject);
+ m_overlappingObjectsIndex.insert(btHashPtr(otherObject), index);
m_hashPairCache->addOverlappingPair(actualThisProxy, otherProxy);
}
}
@@ -82,11 +92,19 @@ void btPairCachingGhostObject::removeOverlappingObjectInternal(btBroadphaseProxy
btAssert(actualThisProxy);
btAssert(otherObject);
- int index = m_overlappingObjects.findLinearSearch(otherObject);
- if (index < m_overlappingObjects.size())
+ int* indexPtr = m_overlappingObjectsIndex.find(btHashPtr(otherObject));
+ if (indexPtr != NULL)
{
- m_overlappingObjects[index] = m_overlappingObjects[m_overlappingObjects.size() - 1];
+ int index = *indexPtr;
+ int lastIndex = m_overlappingObjects.size() - 1;
+ if (index != lastIndex)
+ {
+ btCollisionObject* movedObject = m_overlappingObjects[lastIndex];
+ m_overlappingObjects[index] = movedObject;
+ m_overlappingObjectsIndex.insert(btHashPtr(movedObject), index);
+ }
m_overlappingObjects.pop_back();
+ m_overlappingObjectsIndex.remove(btHashPtr(otherObject));
m_hashPairCache->removeOverlappingPair(actualThisProxy, otherProxy, dispatcher);
}
}

138
defects/bullet3/unit/Bullet3Test.java Normal file
View file

@ -0,0 +1,138 @@
import java.util.*;
/**
* CWE-407 unit test for Bullet Physics btGhostObject defect.
*
* bullet3-0001: src/BulletCollision/CollisionDispatch/btGhostObject.cpp
* btGhostObject::addOverlappingObjectInternal() and removeOverlappingObjectInternal()
* use btAlignedObjectArray::findLinearSearch (O(N) pointer scan) on every broadphase
* pair-update callback. With N objects overlapping a single ghost, each physics tick
* calls addOverlappingObjectInternal N times → O(N²) total per tick.
* The source comment even acknowledges the problem:
* "if this linearSearch becomes too slow (too many overlapping objects)
* we should add a more appropriate data structure"
* Fix: maintain a btHashMap<btHashPtr,int> shadow index → O(1) add/remove.
*
* Severity: HIGH — called every broadphase tick for every ghost-object pair.
* At N=500 overlapping objects the per-tick work grows 500×.
*/
public class Bullet3Test {
// --- defect simulation: linear array dedup ---
static boolean addOverlapping_linear(List<Long> overlapping, long ptr) {
// O(N) scan — models findLinearSearch
for (long p : overlapping) {
if (p == ptr) return false; // already present
}
overlapping.add(ptr);
return true;
}
static boolean removeOverlapping_linear(List<Long> overlapping, long ptr) {
int index = -1;
for (int i = 0; i < overlapping.size(); i++) {
if (overlapping.get(i) == ptr) { index = i; break; }
}
if (index < 0) return false;
// swap with last
overlapping.set(index, overlapping.get(overlapping.size() - 1));
overlapping.remove(overlapping.size() - 1);
return true;
}
// --- fix simulation: HashMap dedup ---
static boolean addOverlapping_hash(List<Long> overlapping, Map<Long,Integer> index, long ptr) {
if (index.containsKey(ptr)) return false; // O(1)
index.put(ptr, overlapping.size());
overlapping.add(ptr);
return true;
}
static boolean removeOverlapping_hash(List<Long> overlapping, Map<Long,Integer> index, long ptr) {
Integer idx = index.get(ptr); // O(1)
if (idx == null) return false;
int lastI = overlapping.size() - 1;
if (idx != lastI) {
long moved = overlapping.get(lastI);
overlapping.set(idx, moved);
index.put(moved, idx);
}
overlapping.remove(lastI);
index.remove(ptr);
return true;
}
// --- correctness check ---
static void assertEqualSets(List<Long> a, List<Long> b, String label) throws Exception {
Set<Long> sa = new HashSet<>(a);
Set<Long> sb = new HashSet<>(b);
if (!sa.equals(sb)) {
throw new Exception("FAIL " + label + ": sets differ. a=" + sa + " b=" + sb);
}
}
static void testBullet30001() throws Exception {
int N = 500;
// Build base state: N objects in ghost overlap list
List<Long> linearList = new ArrayList<>();
List<Long> hashList = new ArrayList<>();
Map<Long,Integer> hashIdx = new HashMap<>();
for (long i = 0; i < N; i++) {
addOverlapping_linear(linearList, i);
addOverlapping_hash(hashList, hashIdx, i);
}
assertEqualSets(linearList, hashList, "initial fill");
// Attempt to re-add all (dedup test) — N² work in linear, O(N) in hash
long linearOps = 0, hashOps = 0;
for (long i = 0; i < N; i++) {
// linear: scan all N before deciding "already there"
boolean found = false;
for (long p : linearList) { linearOps++; if (p == i) { found = true; break; } }
// hash: O(1)
hashOps++;
addOverlapping_hash(hashList, hashIdx, i);
}
assertEqualSets(linearList, hashList, "after re-add");
// Remove even-indexed objects
for (long i = 0; i < N; i += 2) {
removeOverlapping_linear(linearList, i);
removeOverlapping_hash(hashList, hashIdx, i);
}
assertEqualSets(linearList, hashList, "after remove evens");
// Verify index consistency
for (int i = 0; i < hashList.size(); i++) {
long ptr = hashList.get(i);
Integer stored = hashIdx.get(ptr);
if (stored == null || stored != i) {
throw new Exception("FAIL index consistency at i=" + i + " ptr=" + ptr + " stored=" + stored);
}
}
// Performance ratio
double ratio = (double) linearOps / hashOps;
System.out.printf("bullet3-0001 PASS N=%d linear_ops=%d hash_ops=%d ratio=%.1fx%n",
N, linearOps, hashOps, ratio);
if (ratio < 2.0) {
throw new Exception("FAIL: expected ratio >= 2x, got " + ratio);
}
}
public static void main(String[] args) throws Exception {
testBullet30001();
System.out.println("All bullet3 tests PASS");
}
}

52
defects/three.js/patch/three-0001-node-builder-array-includes-set.patch Normal file
View file

@ -0,0 +1,52 @@
# UNDF: (leave blank)
--- a/src/nodes/core/NodeBuilder.js
+++ b/src/nodes/core/NodeBuilder.js
@@ -137,6 +137,13 @@ class NodeBuilder {
* @type {Array<Node>}
*/
this.nodes = [];
+
+ // O(1) membership guard for this.nodes and this.sequentialNodes.
+ // this.nodes.includes(node) is O(N) and is called once per node during
+ // the shader build traversal, making addNode() O(N²) for graphs with N
+ // nodes. A Set<Node> reduces each membership test to O(1), giving O(N)
+ // total build cost. CWE-407.
+ this._nodesSet = new Set();
+ this._sequentialNodesSet = new Set();
/**
* A list of all nodes the builder is processing in sequential order.
@@ -762,8 +762,8 @@ class NodeBuilder {
addNode( node ) {
- if ( this.nodes.includes( node ) === false ) {
+ if ( this._nodesSet.has( node ) === false ) {
this.nodes.push( node );
+ this._nodesSet.add( node );
this.setHashNode( node, node.getHash( this ) );
@@ -785,8 +785,8 @@ class NodeBuilder {
if ( updateBeforeType !== NodeUpdateType.NONE || updateAfterType !== NodeUpdateType.NONE ) {
- if ( this.sequentialNodes.includes( node ) === false ) {
+ if ( this._sequentialNodesSet.has( node ) === false ) {
this.sequentialNodes.push( node );
+ this._sequentialNodesSet.add( node );
}
--- a/src/nodes/core/StackNode.js
+++ b/src/nodes/core/StackNode.js
@@ -382,7 +382,9 @@ class StackNode extends Node {
this._currentNode = null;
- const newNodes = this.nodes.filter( ( node ) => nodes.indexOf( node ) === - 1 );
+ // nodes.indexOf(node) is O(N) inside filter → O(N²) overall.
+ // Use a Set for O(N) total. CWE-407.
+ const nodesSnapshot = new Set( nodes );
+ const newNodes = this.nodes.filter( ( node ) => nodesSnapshot.has( node ) === false );
for ( const node of newNodes ) {

154
defects/three.js/unit/ThreeJsTest.java Normal file
View file

@ -0,0 +1,154 @@
import java.util.*;
/**
* CWE-407 unit test for three.js NodeBuilder defect.
*
* three-0001: src/nodes/core/NodeBuilder.js addNode() / addSequentialNode()
* src/nodes/core/StackNode.js generate()
*
* NodeBuilder.addNode() guards deduplication with:
* if ( this.nodes.includes( node ) === false ) { ... }
* Array.includes() is O(N). It is called once per node during the shader
* build traversal (Node.build → builder.addNode), making the full build
* O(N²) in the number of unique nodes.
*
* addSequentialNode() has the identical pattern with sequentialNodes.
*
* StackNode.generate() compounds this:
* const newNodes = this.nodes.filter( n => nodes.indexOf(n) === -1 );
* filter is O(N), indexOf is O(N) per element → O(N²) to compute the
* newly-added-node delta.
*
* Fix: shadow Array with Set for O(1) has() checks.
*
* Severity: MEDIUM — called during shader compilation (material first-frame
* or material.needsUpdate). Complex scenes with N=300+ nodes (particles,
* post-processing chains) stall on first render due to quadratic build cost.
*
* Speedup: O(N²) → O(N); measured ~250x op-count reduction at N=500.
*/
public class ThreeJsTest {
// --- defect simulation: Array.includes dedup (O(N) per call) ---
static void addNode_array(List<Object> nodes, Object node) {
if (!nodes.contains(node)) { // O(N)
nodes.add(node);
}
}
// Build simulation: traverse N nodes, each calling addNode
static long buildShader_array(List<Object> allNodes) {
List<Object> visited = new ArrayList<>();
long ops = 0;
for (Object node : allNodes) {
// simulate nodes.includes(node): scan visited
boolean found = false;
for (Object v : visited) { ops++; if (v == node) { found = true; break; } }
if (!found) {
visited.add(node);
ops++; // the push
}
}
return ops;
}
// --- fix simulation: Set.has dedup (O(1) per call) ---
static void addNode_set(List<Object> nodes, Set<Object> nodeSet, Object node) {
if (!nodeSet.contains(node)) { // O(1)
nodes.add(node);
nodeSet.add(node);
}
}
static long buildShader_set(List<Object> allNodes) {
List<Object> visited = new ArrayList<>();
Set<Object> visitedSet = new HashSet<>();
long ops = 0;
for (Object node : allNodes) {
ops++; // O(1) set lookup
if (!visitedSet.contains(node)) {
visited.add(node);
visitedSet.add(node);
}
}
return ops;
}
// --- filter delta simulation: nodes.indexOf vs Set.has ---
static long filterDelta_indexOf(List<Object> allNodes, List<Object> snapshot) {
long ops = 0;
List<Object> newNodes = new ArrayList<>();
for (Object node : allNodes) {
// indexOf O(N) per element
boolean found = false;
for (Object s : snapshot) { ops++; if (s == node) { found = true; break; } }
if (!found) newNodes.add(node);
}
return ops;
}
static long filterDelta_set(List<Object> allNodes, List<Object> snapshot) {
long ops = 0;
Set<Object> snapshotSet = new HashSet<>(snapshot);
List<Object> newNodes = new ArrayList<>();
for (Object node : allNodes) {
ops++; // O(1) set lookup
if (!snapshotSet.contains(node)) newNodes.add(node);
}
return ops;
}
static void testThree0001() throws Exception {
int N = 500;
// Build N unique node objects
List<Object> allNodes = new ArrayList<>();
for (int i = 0; i < N; i++) allNodes.add(new Object());
// Shader build dedup
long arrayOps = buildShader_array(allNodes);
long setOps = buildShader_set(allNodes);
double buildRatio = (double) arrayOps / setOps;
System.out.printf("three-0001 addNode N=%d array_ops=%d set_ops=%d ratio=%.1fx%n",
N, arrayOps, setOps, buildRatio);
if (buildRatio < 2.0) {
throw new Exception("FAIL: addNode ratio expected >= 2x, got " + buildRatio);
}
// StackNode.generate filter-delta
// snapshot = first half of nodes, allNodes = all N
List<Object> snapshot = new ArrayList<>(allNodes.subList(0, N / 2));
long idxOps = filterDelta_indexOf(allNodes, snapshot);
long setOps2 = filterDelta_set(allNodes, snapshot);
double filterRatio = (double) idxOps / setOps2;
System.out.printf("three-0001 filter N=%d indexOf_ops=%d set_ops=%d ratio=%.1fx%n",
N, idxOps, setOps2, filterRatio);
if (filterRatio < 2.0) {
throw new Exception("FAIL: filter ratio expected >= 2x, got " + filterRatio);
}
// Correctness: final visited sets must be equal
List<Object> arrayVisited = new ArrayList<>();
List<Object> setVisited = new ArrayList<>();
Set<Object> setMirror = new HashSet<>();
for (Object node : allNodes) {
addNode_array(arrayVisited, node);
addNode_set(setVisited, setMirror, node);
}
if (!new HashSet<>(arrayVisited).equals(new HashSet<>(setVisited))) {
throw new Exception("FAIL: visited sets differ between array and set implementations");
}
System.out.println("three-0001 PASS");
}
public static void main(String[] args) throws Exception {
testThree0001();
System.out.println("All three.js tests PASS");
}
}