[PATCH] memcpy: use bhs/bls instead of bge/blt (CVE-2020-6096) [BZ #25620]

[Carlos O'Donell]

On 4/9/20 12:25 PM, Joseph Myers wrote:
> On Thu, 9 Apr 2020, zhuyan (M) wrote:
> 
>> An exploitable signed comparison vulnerability exists in the ARMv7
>> memcpy() implementation of GNU glibc. Calling memcpy() (on ARMv7
>> targets that utilize the GNU glibc implementation) with a negative
>> value for the 'num' parameter results in a signed comparison
>> vulnerability.
>>
>> If an attacker underflows the 'num' parameter to memcpy(), this
>> vulnerability could lead to undefined behavior such as writing to
>> out-of-bounds memory and potentially remote code execution.
>> Furthermore, this memcpy() implementation allows for program
>> execution to continue in scenarios where a segmentation fault or
>> crash should have occurred. The dangers occur in that subsequent
>> execution and iterations of this code will be executed with this
>> corrupted data.
> 
> I don't object to changing to use unsigned comparisons, since unsigned 
> comparisons are what's logically appropriate here, but the commit message 
> needs to discuss how it's very questionable whether this actually counts 
> as a vulnerability, given that interfaces such as malloc cannot construct 
> objects larger than PTRDIFF_MAX bytes and it's not clear what exactly is 
> permitted to be done with a larger memory region allocated with mmap, so 
> it's not clear if there are any cases where these functions can validly be 
> called with a size argument exceeding PTRDIFF_MAX.
> 
> Any case where a size argument is passed that is larger than the allocated 
> memory region cannot be a security vulnerability in glibc, only in the 
> application passing the bogus size argument (though of course we sometimes 
> do choose to improve hardening against buggy applications).
> 
> Furthermore, there was a comment in the bug saying a new test should be 
> added to the testsuite.  So either the patch should add a test (that fails 
> before and passes after the memcpy change is applied) or the commit 
> message should justify why this is hard to test in the testsuite.

This change should *absolutely* have a test case so we can see if there
are other similar problems with memcpy on other architectures. A synthetic
test case should be possible to construct.

-- 
Cheers,
Carlos.