Bug 514763

Date: 2026-05-21 Target: Kdenlive Master Branch Scope: Deep pattern analysis for memory safety, command injection, and resource exhaustion.
Summary

A comprehensive audit of the Kdenlive has identified 2 critical/high severity vulnerabilities and several code quality improvements.

The most significant finding is a Heap Buffer Overflow in mltdevicecapture.cpp triggered by an integer overflow during image size calculation. Additionally, a Denial of Service (DoS) vector was confirmed in the audio levels generation logic where excessive memory can be allocated based on media inputs.
1. Critical Vulnerabilities
1.1. Heap Buffer Overflow in 

mltdevicecapture.cpp

Severity: Critical Location: 

src/capture/mltdevicecapture.cpp lines 200, 211, 296 Vulnerability Type: Integer Overflow leading to Heap Buffer Overflow

Description: The code calculates the size for a memcpy operation using the expression width * height * 3.

memcpy(qimage.bits(), image, size_t(width * height * 3));

The variables width and height are 

int types (derived from MLT). If specific large values are supplied (e.g., width = 40000, height = 40000), the multiplication width * height * 3 can overflow a 32-bit signed integer, wrapping around to a small positive number or negative number (cast to a large size_t). However, the QImage buffer qimage.bits() is allocated based on the correct or internal properties, but if the source 

image buffer from MLT is larger than the calculated wrapped size, or if the calculation itself mismatches the actual QImage byte count expectations, a crash or memory corruption occurs. Crucially, if the size_t cast happens after the overflow, a very small size might be passed to memcpy, potentially hiding the issue, logic-wise. But slightly different values could cause a heap overflow if the destination buffer is smaller than the source data being copied due to mismatched size logic.

Recommendation: Use size_t (or qint64) for the multiplication to prevent overflow, and validate dimensions against reasonable maximums before allocation.

size_t dataSize = static_cast<size_t>(width) * height * 3;

// Ensure dataSize matches destination capacity

if (dataSize > static_cast<size_t>(qimage.sizeInBytes())) return;

memcpy(qimage.bits(), image, dataSize);
1.2. Denial of Service (Memory Exhaustion) in Audio Generators

Severity: High Location: 

src/jobs/audiolevels/generators.cpp Vulnerability Type: Unbounded Memory Allocation

Description: The application allocates a QVector<int16_t> levels whose size is determined by:

lengthInFrames * AUDIOLEVELS_POINTS_PER_FRAME * channels

    lengthInFrames: Sourced from media duration (can be arbitrarily large).
    channels: Sourced from media metadata (can be manipulated).

A malicious media file claiming to have 100 channels and an extremely long duration could force Kdenlive to attempt a multi-gigabyte allocation, causing the application to crash (OOM) or system instability.

Recommendation: Implement a hard limit on the maximum buffer size. Process audio levels in chunks or stream them rather than loading the entire analysis into a monolithic vector.
2. Security Findings
2.1. Path Traversal Risks

Severity: Low (Mitigated) Analysis: Multiple QFile::open(..., WriteOnly) calls were investigated.

    RenderRequest.cpp: The application constructs filenames using user-supplied guide names. However, correct sanitization replace('/', '_') was found in lines 482-485, preventing directory traversal.
    ArchiveWidget.cpp: Construct paths for project extraction. Reliance on standard KZip/KTar libraries and QDir logic appears standard, though precaution is advised when extracting untrusted archives.

2.2. Command Injection

Severity: Low Analysis:

    QProcess Usage: Extensive use of QProcess for ffmpeg and Python scripts.
    Mitigation: Arguments are generally passed as a list (QStringList), preventing shell injection.
    Risk: RecManager.cpp uses KdenliveSettings::grab_parameters(). If a user is tricked into importing a malicious config file with shell metacharacters in these parameters, it could be a risk, but this requires significant prior compromise (configuration injection).

3. Code Quality & Stability
3.1. Infinite Loops

Status: Safe Identified while(true) loops in 

TimelineController and ManageSubtitles were manually verified to have correct break conditions tied to user interaction, preventing infinite execution.
3.2. Raw Pointers

Status: Acceptable Legacy The codebase uses raw pointers in some legacy areas (AudioCorrelation), but modern components correctly use std::unique_ptr and Qt's parent-child ownership model.

Module: Video Rendering (src/render, src/dialogs/renderwidget.cpp) Focus: Bugs, Performance, Security, and Code Quality
Critical Findings
1. Memory Leak in RenderWidget::slotPrepareExport2
Type: Memory Leak Location: src/dialogs/renderwidget.cpp lines 965-1058 Description: A RenderRequest object is allocated on the heap:
RenderRequest *request = new RenderRequest();
The function uses it to generate jobs and check for errors, but never deletes it. The request object simply leaks every time the user attempts to render. Recommendation: Use std::unique_ptr<RenderRequest> request = std::make_unique<RenderRequest>(); or explicitly delete request; at the end of the function.
2. UI Freezing (Blocking I/O on Main Thread)
Type: Responsiveness / Performance Location: src/dialogs/renderwidget.cpp line 1027 calls request->process() Description: RenderRequest::process() performs heavy I/O operations:

Synchronous XML parsing (doc.setContent).
Massive file writing (QFile::copy, Xml::docContentToFile).
Directory creation. Since slotPrepareExport2 is a Qt slot connected to a UI button, this entire process runs on the Main UI Thread, causing Kdenlive to freeze (become unresponsive) during the "Preparing" phase of large projects. Recommendation: Move RenderRequest::process() to a worker thread (e.g., QtConcurrent::run).

3. Temporary File Leak

Type: Resource Leak Location: src/render/renderrequest.cpp line 339 Description:
QTemporaryFile tmp(...);
tmp.setAutoRemove(false);
The code explicitly disables auto-removal of temporary MLT playlist files so they can be passed to the render process. However, there is no clear mechanism in RenderWidget or RenderJobItem to clean up these files after the render job completes (successfully or failed). Over time, /tmp will fill with .mlt files. Recommendation: Track these files in RenderJobItem and delete them in the destructor or upon job completion/abort.

4. Race Condition in Job Status
Type: Concurrency Bug Location: src/dialogs/renderwidget.cpp (Method checkRenderStatus) Description: checkRenderStatus assumes a job has started if it sets the status to STARTINGJOB. However, startRendering (line 1179) might fail proc.startDetached(), setting the status back to FAILEDJOB. If checkRenderStatus runs concurrently or the status logic is slightly out of sync (e.g. m_renderStatus update timing), the UI might get stuck in a "Rendering" state when no process is actually running.

5. Inefficient XML Deep Copying in Loops
Type: Performance Location: src/render/renderrequest.cpp lines 231, 253, 291 Description: Inside loops iterating over render sections or passes:
QDomDocument sectionDoc = doc.cloneNode(true).toDocument();
This performs a deep copy of the entire DOM tree (project file). For complex projects with thousands of clips, cloning the DOM repeatedly is extremely expensive and memory-intensive. Recommendation: Optimize by using shared data structures or modifying the DOM in place if possible (though difficult with XML). At minimum, audit if the clone is strictly necessary for every iteration.
Architectural & Quality Issues
6. Architectural Violation: UI Logic in Backend Class
Type: Code Smell Location: src/render/renderrequest.cpp lines 372, 384 Description: RenderRequest is a data/logic processing class in src/render. However, it explicitly calls:

QInputDialog::getText

KMessageBox::questionTwoActions This creates a hard dependency on QtWidgets and makes unit testing impossible without visual interruption. It also breaks if the code is moved to a background thread (see finding #2 (closed)). Recommendation: Pass a "UserInteraction" interface or callback to RenderRequest to handle prompts, or resolve these questions in RenderWidget before creating the request.

7. RenderServer Security / Spoofing

Type: Security Location: src/render/renderserver.cpp Description: RenderServer listens on a local socket based on PID. Any local process can connect and send JSON commands. There is no authentication. A malicious local process could send fake setRenderingFinished signals to confuse the user or abort jobs by spoofing the protocol.

8. Unchecked XML Parsing Errors

Type: Stability Location: src/render/renderrequest.cpp line 155 Description: The code calls doc.setContent(&file) (line 158) and returns {} on failure. However, it doesn't propagate the exact parsing error to the user, only a generic failure. This makes debugging corrupt project files during export difficult.

9. Hardcoded Special Case strings

Type: Maintainability Location: src/render/renderrequest.cpp (General) Description: Heavy reliance on magic strings like "kdenlive:uuid", "kdenlive:audio_track", "kdenlive:track_name". Recommendation: Move these to a Definitions or KdenliveNamespace constant file to ensure compile-time safety.

10. Inefficient "Todo" Logic

Type: Efficiency Location: src/render/renderrequest.cpp line 248 Description: A TODO comment explicitly admits to performing an unnecessary file copy: // QFile::copy(job.playlistPath, newJob.playlistPath); // TODO: if we have a single item in sections this is unnesessary and produces just unused files This confirms technical debt that wastes disk I/O.

11. Unsafe Environment Variable Modification

Type: Thread Safety Location: src/main.cpp line 224 Description: qputenv("MLT_REPOSITORY_DENY", ...) modifies global environment variables. While currently in main, if this pattern persists or moves (e.g., enabling/disabling hardware accel at runtime), it is thread-unsafe in C++.

12. Deprecated Qt Attributes

Type: Maintenance Location: src/main.cpp line 220 Description: QCoreApplication::setAttribute(Qt::AA_UseDesktopOpenGL, true); is deprecated in Qt 6 and may vary in behavior across platforms in Qt 5.15+.

13. Hardcoded OS Paths

Type: Portability Location: src/render/renderrequest.cpp lines 309-311 Description: Manual selection of NUL vs /dev/null using #ifdef. Qt provides QIODevice or QFile mechanisms that might abstract this, or use QProcess::setStandardOutputFile(QProcess::Null) instead of passing a path string to an external program if possible.

14. Magic Number in Buffer Calculation (Revisited)

Type: Stability Location: src/jobs/audiolevels/generators.cpp Description: The audit confirmed usage of AUDIOLEVELS_POINTS_PER_FRAME (5) mixed with user-controlled variables. While a DoS risk, it's also a "Coding Issue" where a hardcoded constant drives memory allocation strategy without bounds checking.

15. Lack of "Safe" Overwrite

Type: UX / Data Safety Location: src/dialogs/renderwidget.cpp Description: When checking for existing files, the code asks the user. If they say "Overwrite", it proceeds. However, if the render fails halfway, the original file is already destroyed (truncated). Recommendation: Render to a temporary filename (e.g., video.mp4.part) and rename only on success.