Fix multiple memory safety issues detected by ASan/UBSan/LeakSan
- Fix null pointer dereference in filter_avcolour_space.c when profile is NULL
- Fix Pango/Fontconfig memory leak in producer_pango.c by adding instance counting
- Fix null pointer dereferences in producer_colour.c (two locations)
- Fix null pointer dereference in mlt_producer_set_in_and_out
- Fix null pointer dereference in filter_movit_convert.cpp
- Fix double-free in producer_xml.c by properly handling stack state All fixes have been verified with fuzzing tests. See FIXES_REPORT.md for details, and all of the crash is in the zip fixed_crashes.zip
Some of these checks are good. But the profile checks add a lot of noise to the code. If a service was not initialized with a profile, then the developer is not using the API correctly and we should let them know right away - not mask it and make it more difficult to discover later. How would you feel about adding assert checks for profiles before using them? That would add less conditional checks to the code and give the developer the earliest warning that something is wrong.
@ShadowRider09 could you provide the commands you used to discover these issues? We might consider to put them to CI to prevent regressions...
@ShadowRider09 could you provide the commands you used to discover these issues? We might consider to put them to CI to prevent regressions... I use the Asan+Fussing to find the safty issues, and there is the source code of fuzzer:
#include <stdint.h> #include <stddef.h> #include <string.h> #include <stdlib.h> #include <unistd.h> #include <fcntl.h>
#include "mlt.h"
int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { // Ignore inputs that are too small or too large if (size < 10 || size > 1024 * 1024) { return 0; }
// Initialize MLT (only on the first call)
static int initialized = 0;
if (!initialized) {
// Set the MLT repository path to our compiled version
setenv("MLT_REPOSITORY", "/home/han/mlt/build-asan/out/lib/mlt", 1);
mlt_factory_init(NULL);
initialized = 1;
}
// Create a temporary file to store fuzzing input
char tmpfile[] = "/tmp/fuzz_mlt_XXXXXX";
int fd = mkstemp(tmpfile);
if (fd == -1) {
return 0;
}
// write data
if (write(fd, data, size) != (ssize_t)size) {
close(fd);
unlink(tmpfile);
return 0;
}
close(fd);
// Attempt to load an XML file as a producer
mlt_producer producer = mlt_factory_producer(NULL, "xml", tmpfile);
if (producer != NULL) {
// Attempt to obtain some frames to trigger more code
mlt_frame frame = NULL;
mlt_service_get_frame(MLT_PRODUCER_SERVICE(producer), &frame, 0);
if (frame != NULL) {
mlt_frame_close(frame);
}
// clean
mlt_producer_close(producer);
}
// delete temp files
unlink(tmpfile);
return 0;
}