From 5b3f4d25ea047f14d6485fb6f456cece384d33ee Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Sun, 24 Jun 2012 23:32:50 +0200 Subject: [PATCH] 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. --- ext/standard/base64.c | 4 ++-- 1 file 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 */ -- 2.40.0