Yesterday, Dave Lewis over at LiquidMatrix Security Digest cried foul at Core Security for releasing too much detail about a recent DoS vulnerability they had discovered. His specific gripe was that they provided an IDA Pro excerpt that showed where the vulnerability was triggered. The excerpt is short, so I'll even copy/paste it here:
.text:00405C1B mov esi, [ebp+dwLen] ; Our value from packet ... .text:00405C20 push edi .text:00405C21 test esi, esi ; Check value != 0 ... .text:00405C31 push esi ; Alloc with our length .text:00405C32 mov [ebp+var_4], 0 .text:00405C39 call operator new(uint); Big values return NULL .text:00405C3E mov ecx, esi ; Memcpy with our length .text:00405C40 mov esi, [ebp+pDestionationAddr] .text:00405C43 mov [ebx+4], eax ; new result is used as dest .text:00405C46 mov edi, eax ; address without checks. .text:00405C48 mov eax, ecx .text:00405C4A add esp, 4 .text:00405C4D shr ecx, 2 .text:00405C50 rep movsd ; AV due to invalid .text:00405C52 mov ecx, eax ; destination pointer. .text:00405C54 and ecx, 3
Dave asserts that publishing 16 commented assembly instructions makes this disclosure irresponsible. But look at the code -- it's completely generic, just a textbook example of what it looks like when you forget to check a return value after calling operator new. Sure, Core gives you the exact offsets into the executable, but so what? If I have the binary, then it's not going to be too hard to find the vulnerability anyway. It's not like Core is giving away a proof-of-concept exploit that generates the malformed registration packet required to trigger the DoS. What's more, they provide a detailed timeline going back to January 30th of this year describing exactly how the disclosure process with the vendor transpired. This looks extremely responsible to me; I just can't understand what is "not cool" here. There's another interesting angle to this, completely unrelated to Core's disclosure process. The vulnerability itself is described in the advisory as follows:
Un-authenticated client programs connecting to the service can send a malformed packet that causes a memory allocation operation (a call to new() operator) to fail returning a NULL pointer. Due to a lack of error-checking for the result of the memory allocation operation, the program later tries to use the pointer as a destination for memory copy operation, triggering an access violation error and terminating the service.
This may bring to mind some recent discussions on whether callers of memory allocation functions should check the return value prior to use. To summarize, one camp says "caller should check", the other camp says "callee should exit on allocation failure." This is a gross oversimplification and if you want more detailed arguments, read the other blog posts that I linked to. In this case, if the "exit on failure" approach were taken, the DoS scenario might still happen, whereas if the caller were checking, the error could be handled more gracefully. More fuel for the debate!