libtorrent-0001/qbittorrent: CWE-407 scan — 1 defect, 1 CLEAN
libtorrent-0001: file_storage::get_or_add_path std::find on m_paths vector O(F*P) MEDIUM 250x qBittorrent: CLEAN (QSet/QHash throughout for membership tests)
This commit is contained in:
parent
a0dde8b41a
commit
0b5409ff95
6 changed files with 195 additions and 0 deletions
|
|
@ -0,0 +1,53 @@
|
||||||
|
# UNDF: UNDF-2026-000000777
|
||||||
|
# UNDF: (leave blank)
|
||||||
|
# CWE-407: file_storage::get_or_add_path uses std::find on m_paths vector O(F*P)
|
||||||
|
#
|
||||||
|
# file_storage::get_or_add_path() is called once per file added to a torrent.
|
||||||
|
# For each call it does a linear scan of m_paths (vector<string>) to check for
|
||||||
|
# duplicate directory paths. For a torrent with F files and P unique paths, the
|
||||||
|
# total cost of building the file_storage is O(F*P).
|
||||||
|
#
|
||||||
|
# Large torrents with 50,000+ files across hundreds of directories hit this on
|
||||||
|
# every .torrent parse and metadata exchange. At P=500, F=50000, that's 25M
|
||||||
|
# string comparisons.
|
||||||
|
#
|
||||||
|
# Fix: maintain an unordered_map<string, path_index_t> alongside m_paths for
|
||||||
|
# O(1) lookup. The vector is retained for index-based access.
|
||||||
|
#
|
||||||
|
# Severity: MEDIUM (torrent load/parse path, not per-packet)
|
||||||
|
# Speedup: ~250x at P=500
|
||||||
|
|
||||||
|
--- a/src/file_storage.cpp
|
||||||
|
+++ b/src/file_storage.cpp
|
||||||
|
@@ -210,7 +210,7 @@
|
||||||
|
aux::path_index_t file_storage::get_or_add_path(string_view const path)
|
||||||
|
{
|
||||||
|
- // do we already have this path in the path list?
|
||||||
|
- auto const p = std::find(m_paths.rbegin(), m_paths.rend(), path);
|
||||||
|
+ // O(1) lookup via hash map instead of O(P) linear scan
|
||||||
|
+ auto const it = m_path_index.find(std::string(path));
|
||||||
|
|
||||||
|
- if (p == m_paths.rend())
|
||||||
|
+ if (it == m_path_index.end())
|
||||||
|
{
|
||||||
|
// no, we don't. add it
|
||||||
|
auto const ret = m_paths.end_index();
|
||||||
|
TORRENT_ASSERT(path.size() == 0 || path[0] != '/');
|
||||||
|
m_paths.emplace_back(path.data(), path.size());
|
||||||
|
+ m_path_index.emplace(std::string(path), ret);
|
||||||
|
return ret;
|
||||||
|
}
|
||||||
|
else
|
||||||
|
{
|
||||||
|
- // yes we do. use it
|
||||||
|
- return aux::path_index_t{aux::numeric_cast<std::uint32_t>(
|
||||||
|
- p.base() - m_paths.begin() - 1)};
|
||||||
|
+ return it->second;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
--- a/include/libtorrent/file_storage.hpp
|
||||||
|
+++ b/include/libtorrent/file_storage.hpp
|
||||||
|
@@ (add to private members, near m_paths declaration)
|
||||||
|
+ // O(1) path deduplication index (path string -> path_index_t)
|
||||||
|
+ std::unordered_map<std::string, aux::path_index_t> m_path_index;
|
||||||
Binary file not shown.
BIN
defects/libtorrent/unit/LibtorrentTest$FileStorageFixed.class
Normal file
BIN
defects/libtorrent/unit/LibtorrentTest$FileStorageFixed.class
Normal file
Binary file not shown.
BIN
defects/libtorrent/unit/LibtorrentTest.class
Normal file
BIN
defects/libtorrent/unit/LibtorrentTest.class
Normal file
Binary file not shown.
114
defects/libtorrent/unit/LibtorrentTest.java
Normal file
114
defects/libtorrent/unit/LibtorrentTest.java
Normal file
|
|
@ -0,0 +1,114 @@
|
||||||
|
import java.util.*;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* CWE-407 simulation: libtorrent file_storage::get_or_add_path
|
||||||
|
*
|
||||||
|
* Reproduces the O(F*P) linear-scan deduplication defect in
|
||||||
|
* file_storage::get_or_add_path where std::find scans m_paths
|
||||||
|
* vector for every file added to a torrent.
|
||||||
|
*
|
||||||
|
* libtorrent-0001: get_or_add_path uses std::find on vector O(F*P)
|
||||||
|
* Fix: unordered_map for O(1) lookup → O(F) total
|
||||||
|
*/
|
||||||
|
public class LibtorrentTest {
|
||||||
|
|
||||||
|
// === DEFECTIVE: linear scan dedup (std::find on vector) ===
|
||||||
|
static class FileStorageDefective {
|
||||||
|
private final List<String> paths = new ArrayList<>();
|
||||||
|
|
||||||
|
/** Simulates get_or_add_path with linear scan */
|
||||||
|
int getOrAddPath(String path) {
|
||||||
|
// std::find(m_paths.rbegin(), m_paths.rend(), path)
|
||||||
|
for (int i = paths.size() - 1; i >= 0; i--) {
|
||||||
|
if (paths.get(i).equals(path)) {
|
||||||
|
return i;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
int ret = paths.size();
|
||||||
|
paths.add(path);
|
||||||
|
return ret;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// === FIXED: hash map dedup ===
|
||||||
|
static class FileStorageFixed {
|
||||||
|
private final List<String> paths = new ArrayList<>();
|
||||||
|
private final Map<String, Integer> pathIndex = new HashMap<>();
|
||||||
|
|
||||||
|
int getOrAddPath(String path) {
|
||||||
|
Integer idx = pathIndex.get(path);
|
||||||
|
if (idx != null) return idx;
|
||||||
|
int ret = paths.size();
|
||||||
|
paths.add(path);
|
||||||
|
pathIndex.put(path, ret);
|
||||||
|
return ret;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Simulate adding F files across P unique directory paths.
|
||||||
|
* Each file's directory is looked up via get_or_add_path.
|
||||||
|
*/
|
||||||
|
static long benchmarkDefective(int numFiles, int numPaths) {
|
||||||
|
FileStorageDefective fs = new FileStorageDefective();
|
||||||
|
String[] dirs = new String[numPaths];
|
||||||
|
for (int i = 0; i < numPaths; i++) {
|
||||||
|
dirs[i] = "dir" + i + "/subdir" + (i % 10);
|
||||||
|
}
|
||||||
|
long ops = 0;
|
||||||
|
for (int f = 0; f < numFiles; f++) {
|
||||||
|
String dir = dirs[f % numPaths];
|
||||||
|
// linear scan: worst case scans all P paths
|
||||||
|
for (int i = fs.paths.size() - 1; i >= 0; i--) {
|
||||||
|
ops++;
|
||||||
|
if (fs.paths.get(i).equals(dir)) break;
|
||||||
|
}
|
||||||
|
fs.getOrAddPath(dir);
|
||||||
|
}
|
||||||
|
return ops;
|
||||||
|
}
|
||||||
|
|
||||||
|
static long benchmarkFixed(int numFiles, int numPaths) {
|
||||||
|
FileStorageFixed fs = new FileStorageFixed();
|
||||||
|
String[] dirs = new String[numPaths];
|
||||||
|
for (int i = 0; i < numPaths; i++) {
|
||||||
|
dirs[i] = "dir" + i + "/subdir" + (i % 10);
|
||||||
|
}
|
||||||
|
long ops = 0;
|
||||||
|
for (int f = 0; f < numFiles; f++) {
|
||||||
|
String dir = dirs[f % numPaths];
|
||||||
|
ops++; // O(1) hash lookup
|
||||||
|
fs.getOrAddPath(dir);
|
||||||
|
}
|
||||||
|
return ops;
|
||||||
|
}
|
||||||
|
|
||||||
|
public static void main(String[] args) {
|
||||||
|
System.out.println("=== LibtorrentTest: CWE-407 file_storage::get_or_add_path ===\n");
|
||||||
|
|
||||||
|
// libtorrent-0001: get_or_add_path linear dedup
|
||||||
|
int numFiles = 50000;
|
||||||
|
int numPaths = 500;
|
||||||
|
long defectOps = benchmarkDefective(numFiles, numPaths);
|
||||||
|
long fixedOps = benchmarkFixed(numFiles, numPaths);
|
||||||
|
double ratio = (double) defectOps / fixedOps;
|
||||||
|
|
||||||
|
System.out.printf("libtorrent-0001: get_or_add_path linear path dedup%n");
|
||||||
|
System.out.printf(" F=%d files, P=%d unique paths%n", numFiles, numPaths);
|
||||||
|
System.out.printf(" defect ops: %,d%n", defectOps);
|
||||||
|
System.out.printf(" fixed ops: %,d%n", fixedOps);
|
||||||
|
System.out.printf(" ratio: %.1fx%n", ratio);
|
||||||
|
|
||||||
|
boolean pass = ratio > 10.0;
|
||||||
|
System.out.printf(" result: %s%n%n", pass ? "PASS" : "FAIL");
|
||||||
|
|
||||||
|
// Summary
|
||||||
|
System.out.println("=== SUMMARY ===");
|
||||||
|
System.out.printf("libtorrent-0001 get_or_add_path O(F*P) -> O(F): %s (%.1fx)%n",
|
||||||
|
pass ? "PASS" : "FAIL", ratio);
|
||||||
|
|
||||||
|
if (!pass) {
|
||||||
|
System.exit(1);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
28
defects/qbittorrent/patch/CLEAN.md
Normal file
28
defects/qbittorrent/patch/CLEAN.md
Normal file
|
|
@ -0,0 +1,28 @@
|
||||||
|
# qBittorrent — CWE-407 Scan Result: CLEAN
|
||||||
|
|
||||||
|
Scanned: 2026-03-30
|
||||||
|
Target: qBittorrent (C++/Qt)
|
||||||
|
Source: https://github.com/qbittorrent/qBittorrent
|
||||||
|
|
||||||
|
## Scope
|
||||||
|
|
||||||
|
- `src/base/bittorrent/` — session, torrent impl, tracker, peer management
|
||||||
|
- `src/base/rss/` — RSS auto-downloader, parser, feed management
|
||||||
|
- `src/base/search/` — search plugin manager
|
||||||
|
- `src/gui/` — transfer list, tracker list, tag/category filtering
|
||||||
|
- `src/webui/` — API controllers, sync controller
|
||||||
|
|
||||||
|
## Finding
|
||||||
|
|
||||||
|
CLEAN. qBittorrent uses appropriate data structures throughout:
|
||||||
|
|
||||||
|
- **Tags**: `OrderedSet<Tag>` (std::set) — O(log N) membership
|
||||||
|
- **Categories**: `QHash<QString, CategoryOptions>` — O(1) membership
|
||||||
|
- **Torrents**: `QHash<TorrentID, TorrentImpl*>` — O(1) lookup
|
||||||
|
- **Banned IPs**: sorted `QStringList` but only accessed from user actions, not hot paths
|
||||||
|
- **Trackers**: `QHash`/`QSet` for tracker host dedup in filter widgets
|
||||||
|
- **RSS article IDs**: `QSet<QString>` — O(1) dedup
|
||||||
|
- **Search disabled plugins**: `QStringList` with O(N) contains, but N < 50 always
|
||||||
|
|
||||||
|
No QList/QVector linear membership test found in any hot loop or
|
||||||
|
per-packet/per-torrent-update path.
|
||||||
Loading…
Add table
Add a link
Reference in a new issue