Fix memory allocation checks for base64 encode

base64_encode used safe_emalloc, but one of the arguments was derived from a
multiplication, thus making the allocation unsafe again.

There was a size check in place, but it was off by a factor of two as it
didn't account for the signedness of the integer type.

The unsafe allocation is not exploitable, but still causes funny behavior
when the sized overflows into a negative number.

To fix the issue the *4 factor is moved into the size argument (where it is
known to be safe), so safe_emalloc can carry out the multiplication.

The size check is removed as it doesn't really make sense once safe_emalloc
works correctly. (Would only cause base64_encode to silently return false
instead of throwing an error. Also could cause problems with other uses of
the base64 encoding API, which all don't check for a NULL return value.)

Furthermore the (length + 2) < 0 check is replaced with just length < 0.
Allowing lengths -2 and -1 doesn't make sense semantically and also is not
honored in the following code (negative length would access unallocated
memory.)

Actually the length < 0 check doesn't make sense altogether, but I left it
there just to be safe.

1 files changed, 2 insertions, 2 deletions

diff --git a/ext/standard/base64.c b/ext/standard/base64.c
index 9e9c36250c..d78cb244c5 100644
--- a/ext/standard/base64.c
+++ b/ext/standard/base64.c
@@ -59,14 +59,14 @@ PHPAPI unsigned char *php_base64_encode(const unsigned char *str, int length, in
 	unsigned char *p;
 	unsigned char *result;
 
-	if ((length + 2) < 0 || ((length + 2) / 3) >= (1 << (sizeof(int) * 8 - 2))) {
+	if (length < 0) {
 		if (ret_length != NULL) {
 			*ret_length = 0;
 		}
 		return NULL;
 	}
 
-	result = (unsigned char *)safe_emalloc(((length + 2) / 3) * 4, sizeof(char), 1);
+	result = (unsigned char *) safe_emalloc((length + 2) / 3, 4 * sizeof(char), 1);
 	p = result;
 
 	while (length > 2) { /* keep going until we have less than 24 bits */