I don't understand patch p6 well enough to say how
serious it is (depends on where the invalid pointer being dereferenced
comes from).
As far as I know, it is just a NULL pointer. Johannes did the analysis
and might be able to elaborate (CCed).
Correct. I am not sure if it is possible at all to trigger that field to
be NULL in libopenmpt but it is possible in OpenMPT in some live editing
situations. I cannot be 100% sure if libopenmpt would ever be able to
trigger this crash but it should be obvious that adding a null-pointer
check should do no harm to the library.
p3-excessive-cpu-consumption-on-malformed-files-dmf-mdl.patch
p5-excessive-cpu-consumption-on-malformed-files-ams.patch
Are these actually security bugs? As long as the code finishes in a
reasonable amount of time and produces the right results, then there's
not much harm in leaving the code as it is.
Again, Johannes knows more about these.
I guess it depends on what you define as "reasonable". Depending on the
malformed file and setup, they may take minutes to load (given that
enough (virtual) memory is available to load all the truncated samples).
The test cases that were generated by American Fuzzy Lop were about 5KB
in size and took about 10 seconds to load on my machine, which I would
say is quite excessive for such a tiny file, but those examples can be
modified (by adding more malformed sample slots) to extend the loading
time from seconds to minutes while still being about 5KB.
To give some perspective, I don't just use libopenmpt on Desktop systems
but also to scan module data in a server application where users can
upload their own modules, and if a user could keep a server request busy
for a minute (while also wasting tons of memory and CPU time), that
server could be DOSed very easily. Thus I'd strongly suggest to include
those two patches.
Cheers,
Johannes
