SIGSEGV crash in main()

[fdegros]

Our crash reporting system indicates that the main source of rar2fs crashes is a specific instruction in its main() function. I believe stack traces are consistent and accurate, and point to the line 1820 in rar2fs.c:

        if (arc->hdr.Flags & RHDF_ENCRYPTED) {

The cause of these crashes is a SIGSEGV. This seems to indicate that the arc pointer is either NULL or invalid, and dereferencing it leads to a segmentation fault.

I don't have a test case reproducing this crash at hand. My interpretation is only based on these automated crash reports.

These crashes don't seem too frequent, but they happen every now and then on several architectures (AMD-64 and ARM-32).

[hasse69]

Thanks for the issue report.

If the cause of this is what I think it is, this must be considered extremely rare.

It would either be due to lack of RAM and failure to allocate memory that in current design could still produce a pointer that is not NULL but still not valid. This is due to how C++ operator new by default works since the time exceptions were introduced. Since we have no dump of the stack frame data it is hard to say if that was the case or not. In this case checking for NULL would not help.

Secondly and probably a lot more likely but still must be considered extremely rare is that the function involved returns _ERAREOPEN without first having allocated this memory in the first place. This can only happen in a very specific branch in the unrar library that I must have overlooked or it was introduced in a later version of it. It covers a very old RAR compression format/version that had a flaw in the way length of each file was calculated. It could result in that an entire file fitted in one volume but still indicated in the header that it continued in the next. Such archives must not be allowed to be opened and hence an error is thrown. But all this is fine, a simple test for NULL would cure that since pointer is initialized to 0 and if no allocation is performed it will remain 0 across the function call.

Thus to solve both these scenarios the solution is to:

• check for NULL where relevant
• call operator new with the (std:nothrow) argument

The last change is needed and cannot be solved in the catch clause. Since we could catch exceptions thrown much deeper down the stack we must be able to rely on that the pointer sent as argument to the function can be safely freed if it is not NULL.

While fixing these two issues I also took some time to change slightly in the _collectfiles() function to make it more robust and also more deterministic. It should resolve some other potential crashes for obfuscated archive names.