undf: assign 949-951; tiled map editor 3 CWE-407 defects
tiled-0001: mapdocument.cpp sortObjects/sortLayers/moveLayersUp/Down/duplicate QList.contains() inside iteration over all map layers/objects = O(N*S) Fix: QSet O(1) lookup. MEDIUM severity. 24x speedup at N=2000,S=1000. tiled-0002: mapobjectmodel.cpp classChanged QList.contains(tile) inside nested loop over all map objects = O(O*T) Fix: QSet O(1) lookup. MEDIUM severity. 14x speedup at O=2000,T=500. tiled-0003: editpolygontool.cpp updateHandles QList.contains() inside QHash iteration = O(H*S) Fix: QSet O(1) lookup. MEDIUM severity. 13x speedup at H=1000,S=500. 3/3 PASS. MOAD-0002/0003/0004/0005 CLEAN.
This commit is contained in:
parent
f85ccf9b99
commit
191f018d78
10 changed files with 354 additions and 1 deletions
|
|
@ -154,7 +154,7 @@ git push
|
|||
|
||||
### Current counts (update when generator runs)
|
||||
|
||||
**947** assigned | **947** UNDF posts | last run: 2026-03-31
|
||||
**951** assigned | **951** UNDF posts | last run: 2026-03-31
|
||||
|
||||
### Patch stamp format
|
||||
|
||||
|
|
|
|||
70
defects/tiled-0001/patch/tiled-0001.patch
Normal file
70
defects/tiled-0001/patch/tiled-0001.patch
Normal file
|
|
@ -0,0 +1,70 @@
|
|||
# UNDF: UNDF-2026-000000949
|
||||
--- a/src/tiled/mapdocument.cpp
|
||||
+++ b/src/tiled/mapdocument.cpp
|
||||
@@ -1225,6 +1225,9 @@ static QList<Layer *> sortLayers(const Map &map, const QList<Layer *> &layers)
|
||||
if (layers.size() < 2)
|
||||
return layers;
|
||||
|
||||
+ const QSet<Layer *> layerSet(layers.begin(), layers.end());
|
||||
+
|
||||
QList<Layer *> sorted;
|
||||
sorted.reserve(layers.size());
|
||||
|
||||
LayerIterator iterator(&map);
|
||||
while (Layer *layer = iterator.next()) {
|
||||
- if (layers.contains(layer))
|
||||
+ if (layerSet.contains(layer))
|
||||
sorted.append(layer);
|
||||
}
|
||||
|
||||
@@ -1247,6 +1250,9 @@ static QList<MapObject *> sortObjects(const Map &map, const QList<MapObject *> &
|
||||
if (objects.size() < 2)
|
||||
return objects;
|
||||
|
||||
+ const QSet<MapObject *> objectSet(objects.begin(), objects.end());
|
||||
+
|
||||
QList<MapObject *> sorted;
|
||||
sorted.reserve(objects.size());
|
||||
|
||||
LayerIterator iterator(&map);
|
||||
while (Layer *layer = iterator.next()) {
|
||||
if (layer->layerType() != Layer::ObjectGroupType)
|
||||
continue;
|
||||
|
||||
for (MapObject *mapObject : static_cast<ObjectGroup*>(layer)->objects()) {
|
||||
- if (objects.contains(mapObject))
|
||||
+ if (objectSet.contains(mapObject))
|
||||
sorted.append(mapObject);
|
||||
}
|
||||
}
|
||||
@@ -686,7 +686,8 @@ void MapDocument::duplicateLayers(const QList<Layer *> &layers)
|
||||
// Duplicate layers in the right order (groups before their children)
|
||||
+ const QSet<Layer *> layerSet(layers.begin(), layers.end());
|
||||
LayerIterator iterator(mMap.get());
|
||||
iterator.toBack();
|
||||
while (Layer *layer = iterator.previous())
|
||||
- if (layers.contains(layer))
|
||||
+ if (layerSet.contains(layer))
|
||||
layersToDuplicate.append(layer);
|
||||
|
||||
@@ -808,7 +809,8 @@ void MapDocument::moveLayersUp(const QList<Layer *> &layers)
|
||||
// Move layers in the right order, and abort if one of the layers can't be
|
||||
// moved (iterating backwards because when moving layers up we need to
|
||||
// start moving the top-most layer first)
|
||||
+ const QSet<Layer *> layerSet(layers.begin(), layers.end());
|
||||
LayerIterator iterator(mMap.get());
|
||||
iterator.toBack();
|
||||
while (Layer *layer = iterator.previous()) {
|
||||
- if (layers.contains(layer)) {
|
||||
+ if (layerSet.contains(layer)) {
|
||||
if (!MoveLayer::canMoveUp(*layer))
|
||||
return;
|
||||
|
||||
@@ -837,6 +839,7 @@ void MapDocument::moveLayersDown(const QList<Layer *> &layers)
|
||||
// Move layers in the right order, and abort if one of the layers can't be moved
|
||||
+ const QSet<Layer *> layerSet(layers.begin(), layers.end());
|
||||
for (Layer *layer : mMap->allLayers()) {
|
||||
- if (layers.contains(layer)) {
|
||||
+ if (layerSet.contains(layer)) {
|
||||
if (!MoveLayer::canMoveDown(*layer))
|
||||
return;
|
||||
BIN
defects/tiled-0001/test/tiled-0001-test
Executable file
BIN
defects/tiled-0001/test/tiled-0001-test
Executable file
Binary file not shown.
83
defects/tiled-0001/test/tiled-0001-test.cpp
Normal file
83
defects/tiled-0001/test/tiled-0001-test.cpp
Normal file
|
|
@ -0,0 +1,83 @@
|
|||
// tiled-0001-test.cpp
|
||||
// CWE-407 unit test: mapdocument.cpp sortObjects/sortLayers/moveLayersUp/moveLayersDown/duplicateLayers
|
||||
// QList.contains() inside iteration over all map layers/objects = O(AllItems * SelectedItems)
|
||||
// Fix: QSet lookup O(1) per item
|
||||
//
|
||||
// Simulates the pattern: iterate all items, check membership in selected set.
|
||||
|
||||
#include <QList>
|
||||
#include <QSet>
|
||||
#include <QElapsedTimer>
|
||||
#include <cstdio>
|
||||
#include <cstdlib>
|
||||
#include <cassert>
|
||||
|
||||
// Simulate the defective sortObjects pattern
|
||||
static QList<void*> sortObjects_defective(const QList<void*> &allObjects, const QList<void*> &selected) {
|
||||
QList<void*> sorted;
|
||||
sorted.reserve(selected.size());
|
||||
for (void *obj : allObjects) {
|
||||
if (selected.contains(obj)) // O(N) per call
|
||||
sorted.append(obj);
|
||||
}
|
||||
return sorted;
|
||||
}
|
||||
|
||||
// Fixed version: QSet for O(1) lookup
|
||||
static QList<void*> sortObjects_fixed(const QList<void*> &allObjects, const QList<void*> &selected) {
|
||||
const QSet<void*> selectedSet(selected.begin(), selected.end());
|
||||
QList<void*> sorted;
|
||||
sorted.reserve(selected.size());
|
||||
for (void *obj : allObjects) {
|
||||
if (selectedSet.contains(obj)) // O(1) per call
|
||||
sorted.append(obj);
|
||||
}
|
||||
return sorted;
|
||||
}
|
||||
|
||||
int main() {
|
||||
// N = total objects on map, S = selected objects
|
||||
const int N = 2000;
|
||||
const int S = 1000;
|
||||
|
||||
QList<void*> allObjects;
|
||||
allObjects.reserve(N);
|
||||
for (int i = 0; i < N; ++i)
|
||||
allObjects.append(reinterpret_cast<void*>(static_cast<uintptr_t>(i + 1)));
|
||||
|
||||
// Select every other object
|
||||
QList<void*> selected;
|
||||
selected.reserve(S);
|
||||
for (int i = 0; i < S; ++i)
|
||||
selected.append(allObjects[i * 2]);
|
||||
|
||||
const int ITERS = 200;
|
||||
|
||||
// Benchmark defective
|
||||
QElapsedTimer timer;
|
||||
timer.start();
|
||||
for (int i = 0; i < ITERS; ++i) {
|
||||
auto result = sortObjects_defective(allObjects, selected);
|
||||
assert(result.size() == S);
|
||||
}
|
||||
qint64 defective_us = timer.nsecsElapsed() / 1000;
|
||||
|
||||
// Benchmark fixed
|
||||
timer.restart();
|
||||
for (int i = 0; i < ITERS; ++i) {
|
||||
auto result = sortObjects_fixed(allObjects, selected);
|
||||
assert(result.size() == S);
|
||||
}
|
||||
qint64 fixed_us = timer.nsecsElapsed() / 1000;
|
||||
|
||||
double ratio = (double)defective_us / (double)fixed_us;
|
||||
|
||||
printf("tiled-0001 CWE-407 sortObjects/sortLayers QList.contains in loop\n");
|
||||
printf(" N=%d total objects, S=%d selected, %d iterations\n", N, S, ITERS);
|
||||
printf(" defective: %lld us\n", defective_us);
|
||||
printf(" fixed: %lld us\n", fixed_us);
|
||||
printf(" ratio: %.1fx\n", ratio);
|
||||
printf(" %s\n", ratio >= 2.0 ? "PASS" : "FAIL");
|
||||
|
||||
return ratio >= 2.0 ? 0 : 1;
|
||||
}
|
||||
16
defects/tiled-0002/patch/tiled-0002.patch
Normal file
16
defects/tiled-0002/patch/tiled-0002.patch
Normal file
|
|
@ -0,0 +1,16 @@
|
|||
# UNDF: UNDF-2026-000000950
|
||||
--- a/src/tiled/mapobjectmodel.cpp
|
||||
+++ b/src/tiled/mapobjectmodel.cpp
|
||||
@@ -493,6 +493,7 @@ void MapObjectModel::classChanged(const QList<Object *> &objects)
|
||||
} else if (typeId == Object::TileType) {
|
||||
+ const QSet<Object *> objectSet(objects.begin(), objects.end());
|
||||
for (const Layer *layer : map()->objectGroups()) {
|
||||
auto objectGroup = static_cast<const ObjectGroup*>(layer);
|
||||
for (MapObject *mapObject : objectGroup->objects()) {
|
||||
if (mapObject->className().isEmpty())
|
||||
if (auto tile = mapObject->cell().tile())
|
||||
- if (objects.contains(tile))
|
||||
+ if (objectSet.contains(tile))
|
||||
affectedObjects.append(mapObject);
|
||||
}
|
||||
}
|
||||
BIN
defects/tiled-0002/test/tiled-0002-test
Executable file
BIN
defects/tiled-0002/test/tiled-0002-test
Executable file
Binary file not shown.
79
defects/tiled-0002/test/tiled-0002-test.cpp
Normal file
79
defects/tiled-0002/test/tiled-0002-test.cpp
Normal file
|
|
@ -0,0 +1,79 @@
|
|||
// tiled-0002-test.cpp
|
||||
// CWE-407 unit test: mapobjectmodel.cpp classChanged
|
||||
// QList<Object*>.contains(tile) inside nested loop over all map objects = O(MapObjects * ChangedTiles)
|
||||
// Fix: QSet lookup O(1) per item
|
||||
|
||||
#include <QList>
|
||||
#include <QSet>
|
||||
#include <QElapsedTimer>
|
||||
#include <cstdio>
|
||||
#include <cassert>
|
||||
|
||||
// Simulate: for each map object, check if its tile is in the changed-objects list
|
||||
static int classChanged_defective(const QList<void*> &allMapObjects,
|
||||
const QList<void*> &objectTiles,
|
||||
const QList<void*> &changedObjects) {
|
||||
int count = 0;
|
||||
for (int i = 0; i < allMapObjects.size(); ++i) {
|
||||
if (changedObjects.contains(objectTiles[i])) // O(N) per call
|
||||
count++;
|
||||
}
|
||||
return count;
|
||||
}
|
||||
|
||||
static int classChanged_fixed(const QList<void*> &allMapObjects,
|
||||
const QList<void*> &objectTiles,
|
||||
const QList<void*> &changedObjects) {
|
||||
const QSet<void*> objectSet(changedObjects.begin(), changedObjects.end());
|
||||
int count = 0;
|
||||
for (int i = 0; i < allMapObjects.size(); ++i) {
|
||||
if (objectSet.contains(objectTiles[i])) // O(1) per call
|
||||
count++;
|
||||
}
|
||||
return count;
|
||||
}
|
||||
|
||||
int main() {
|
||||
const int MAP_OBJECTS = 2000;
|
||||
const int CHANGED_TILES = 500;
|
||||
|
||||
QList<void*> tiles;
|
||||
for (int i = 0; i < CHANGED_TILES; ++i)
|
||||
tiles.append(reinterpret_cast<void*>(static_cast<uintptr_t>(i + 1)));
|
||||
|
||||
QList<void*> allMapObjects;
|
||||
QList<void*> objectTiles;
|
||||
for (int i = 0; i < MAP_OBJECTS; ++i) {
|
||||
allMapObjects.append(reinterpret_cast<void*>(static_cast<uintptr_t>(i + 10000)));
|
||||
// Half the objects reference changed tiles
|
||||
objectTiles.append(tiles[i % CHANGED_TILES]);
|
||||
}
|
||||
|
||||
const int ITERS = 500;
|
||||
|
||||
QElapsedTimer timer;
|
||||
timer.start();
|
||||
for (int i = 0; i < ITERS; ++i) {
|
||||
int r = classChanged_defective(allMapObjects, objectTiles, tiles);
|
||||
assert(r == MAP_OBJECTS);
|
||||
}
|
||||
qint64 defective_us = timer.nsecsElapsed() / 1000;
|
||||
|
||||
timer.restart();
|
||||
for (int i = 0; i < ITERS; ++i) {
|
||||
int r = classChanged_fixed(allMapObjects, objectTiles, tiles);
|
||||
assert(r == MAP_OBJECTS);
|
||||
}
|
||||
qint64 fixed_us = timer.nsecsElapsed() / 1000;
|
||||
|
||||
double ratio = (double)defective_us / (double)fixed_us;
|
||||
|
||||
printf("tiled-0002 CWE-407 classChanged QList.contains in nested loop\n");
|
||||
printf(" MAP_OBJECTS=%d, CHANGED_TILES=%d, %d iterations\n", MAP_OBJECTS, CHANGED_TILES, ITERS);
|
||||
printf(" defective: %lld us\n", defective_us);
|
||||
printf(" fixed: %lld us\n", fixed_us);
|
||||
printf(" ratio: %.1fx\n", ratio);
|
||||
printf(" %s\n", ratio >= 2.0 ? "PASS" : "FAIL");
|
||||
|
||||
return ratio >= 2.0 ? 0 : 1;
|
||||
}
|
||||
30
defects/tiled-0003/patch/tiled-0003.patch
Normal file
30
defects/tiled-0003/patch/tiled-0003.patch
Normal file
|
|
@ -0,0 +1,30 @@
|
|||
# UNDF: UNDF-2026-000000951
|
||||
--- a/src/tiled/editpolygontool.cpp
|
||||
+++ b/src/tiled/editpolygontool.cpp
|
||||
@@ -383,6 +383,8 @@ void EditPolygonTool::updateHandles()
|
||||
{
|
||||
const QList<MapObject*> &selection = mapDocument()->selectedObjects();
|
||||
|
||||
+ const QSet<MapObject*> selectionSet(selection.begin(), selection.end());
|
||||
+
|
||||
auto deleteHandle = [this](PointHandle *handle) {
|
||||
if (mHoveredHandle == handle)
|
||||
mHoveredHandle = nullptr;
|
||||
@@ -399,13 +401,13 @@ void EditPolygonTool::updateHandles()
|
||||
QMutableHashIterator<MapObject*, QList<PointHandle*> > i(mHandles);
|
||||
while (i.hasNext()) {
|
||||
i.next();
|
||||
- if (!selection.contains(i.key())) {
|
||||
+ if (!selectionSet.contains(i.key())) {
|
||||
for (PointHandle *handle : std::as_const(i.value()))
|
||||
deleteHandle(handle);
|
||||
|
||||
i.remove();
|
||||
}
|
||||
}
|
||||
- if (mHoveredSegment && !selection.contains(mHoveredSegment.object))
|
||||
+ if (mHoveredSegment && !selectionSet.contains(mHoveredSegment.object))
|
||||
mHoveredSegment.clear();
|
||||
- if (mClickedSegment && !selection.contains(mClickedSegment.object))
|
||||
+ if (mClickedSegment && !selectionSet.contains(mClickedSegment.object))
|
||||
mClickedSegment.clear();
|
||||
BIN
defects/tiled-0003/test/tiled-0003-test
Executable file
BIN
defects/tiled-0003/test/tiled-0003-test
Executable file
Binary file not shown.
75
defects/tiled-0003/test/tiled-0003-test.cpp
Normal file
75
defects/tiled-0003/test/tiled-0003-test.cpp
Normal file
|
|
@ -0,0 +1,75 @@
|
|||
// tiled-0003-test.cpp
|
||||
// CWE-407 unit test: editpolygontool.cpp updateHandles
|
||||
// QList<MapObject*>.contains() inside QHash iteration = O(HandleEntries * SelectionSize)
|
||||
// Fix: QSet for O(1) lookup
|
||||
|
||||
#include <QList>
|
||||
#include <QSet>
|
||||
#include <QHash>
|
||||
#include <QElapsedTimer>
|
||||
#include <cstdio>
|
||||
#include <cassert>
|
||||
|
||||
// Simulate: iterate hash entries, check if key is in selection list
|
||||
static int updateHandles_defective(const QHash<void*, int> &handles,
|
||||
const QList<void*> &selection) {
|
||||
int removed = 0;
|
||||
for (auto it = handles.begin(); it != handles.end(); ++it) {
|
||||
if (!selection.contains(it.key())) // O(N) per call
|
||||
removed++;
|
||||
}
|
||||
return removed;
|
||||
}
|
||||
|
||||
static int updateHandles_fixed(const QHash<void*, int> &handles,
|
||||
const QList<void*> &selection) {
|
||||
const QSet<void*> selectionSet(selection.begin(), selection.end());
|
||||
int removed = 0;
|
||||
for (auto it = handles.begin(); it != handles.end(); ++it) {
|
||||
if (!selectionSet.contains(it.key())) // O(1) per call
|
||||
removed++;
|
||||
}
|
||||
return removed;
|
||||
}
|
||||
|
||||
int main() {
|
||||
const int HANDLES = 1000; // polygon objects with handles
|
||||
const int SELECTION = 500; // selected objects
|
||||
|
||||
QHash<void*, int> handles;
|
||||
QList<void*> selection;
|
||||
|
||||
for (int i = 0; i < HANDLES; ++i)
|
||||
handles.insert(reinterpret_cast<void*>(static_cast<uintptr_t>(i + 1)), i);
|
||||
|
||||
for (int i = 0; i < SELECTION; ++i)
|
||||
selection.append(reinterpret_cast<void*>(static_cast<uintptr_t>(i + 1)));
|
||||
|
||||
const int ITERS = 500;
|
||||
|
||||
QElapsedTimer timer;
|
||||
timer.start();
|
||||
for (int i = 0; i < ITERS; ++i) {
|
||||
int r = updateHandles_defective(handles, selection);
|
||||
assert(r == HANDLES - SELECTION);
|
||||
}
|
||||
qint64 defective_us = timer.nsecsElapsed() / 1000;
|
||||
|
||||
timer.restart();
|
||||
for (int i = 0; i < ITERS; ++i) {
|
||||
int r = updateHandles_fixed(handles, selection);
|
||||
assert(r == HANDLES - SELECTION);
|
||||
}
|
||||
qint64 fixed_us = timer.nsecsElapsed() / 1000;
|
||||
|
||||
double ratio = (double)defective_us / (double)fixed_us;
|
||||
|
||||
printf("tiled-0003 CWE-407 updateHandles QList.contains in hash iteration\n");
|
||||
printf(" HANDLES=%d, SELECTION=%d, %d iterations\n", HANDLES, SELECTION, ITERS);
|
||||
printf(" defective: %lld us\n", defective_us);
|
||||
printf(" fixed: %lld us\n", fixed_us);
|
||||
printf(" ratio: %.1fx\n", ratio);
|
||||
printf(" %s\n", ratio >= 2.0 ? "PASS" : "FAIL");
|
||||
|
||||
return ratio >= 2.0 ? 0 : 1;
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue