Skip to content

Commit a90bc3c

Browse files
committed
Introduction of timing attack safe bcmp implementation.
Nothing new but to refactor usage b/w hash and password extensions but using volatile pointers to be a bit safer, allowing to expand its usage eventually.
1 parent 5f900b3 commit a90bc3c

File tree

5 files changed

+43
-9
lines changed

5 files changed

+43
-9
lines changed

configure.ac

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1437,7 +1437,7 @@ PHP_ADD_SOURCES(main, main.c snprintf.c spprintf.c \
14371437
fopen_wrappers.c alloca.c php_scandir.c \
14381438
php_ini.c SAPI.c rfc1867.c php_content_types.c strlcpy.c \
14391439
strlcat.c explicit_bzero.c reentrancy.c php_variables.c php_ticks.c \
1440-
network.c php_open_temporary_file.c \
1440+
network.c php_open_temporary_file.c safe_bcmp.c \
14411441
output.c getopt.c php_syslog.c, -DZEND_ENABLE_STATIC_TSRMLS_CACHE=1)
14421442

14431443
PHP_ADD_SOURCES_X(main, fastcgi.c, -DZEND_ENABLE_STATIC_TSRMLS_CACHE=1, PHP_FASTCGI_OBJS, no)

ext/hash/hash.c

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -853,7 +853,6 @@ PHP_FUNCTION(hash_equals)
853853
zval *known_zval, *user_zval;
854854
char *known_str, *user_str;
855855
int result = 0;
856-
size_t j;
857856

858857
if (zend_parse_parameters(ZEND_NUM_ARGS(), "zz", &known_zval, &user_zval) == FAILURE) {
859858
RETURN_THROWS();
@@ -878,9 +877,7 @@ PHP_FUNCTION(hash_equals)
878877
user_str = Z_STRVAL_P(user_zval);
879878

880879
/* This is security sensitive code. Do not optimize this for speed. */
881-
for (j = 0; j < Z_STRLEN_P(known_zval); j++) {
882-
result |= known_str[j] ^ user_str[j];
883-
}
880+
result = php_safe_bcmp(known_str, user_str, Z_STRLEN_P(known_zval));
884881

885882
RETURN_BOOL(0 == result);
886883
}

ext/standard/password.c

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,6 @@ static zend_bool php_password_bcrypt_needs_rehash(const zend_string *hash, zend_
156156
}
157157

158158
static zend_bool php_password_bcrypt_verify(const zend_string *password, const zend_string *hash) {
159-
size_t i;
160159
int status = 0;
161160
zend_string *ret = php_crypt(ZSTR_VAL(password), (int)ZSTR_LEN(password), ZSTR_VAL(hash), (int)ZSTR_LEN(hash), 1);
162161

@@ -173,9 +172,7 @@ static zend_bool php_password_bcrypt_verify(const zend_string *password, const z
173172
* resistance towards timing attacks. This is a constant time
174173
* equality check that will always check every byte of both
175174
* values. */
176-
for (i = 0; i < ZSTR_LEN(hash); i++) {
177-
status |= (ZSTR_VAL(ret)[i] ^ ZSTR_VAL(hash)[i]);
178-
}
175+
status = php_safe_bcmp(ZSTR_VAL(ret), ZSTR_VAL(hash), ZSTR_LEN(hash));
179176

180177
zend_string_free(ret);
181178
return status == 0;

main/php.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,10 @@ END_EXTERN_C()
187187
#define explicit_bzero php_explicit_bzero
188188
#endif
189189

190+
BEGIN_EXTERN_C()
191+
PHPAPI int php_safe_bcmp(const void *a, const void *b, size_t siz);
192+
END_EXTERN_C()
193+
190194
#ifndef HAVE_STRTOK_R
191195
BEGIN_EXTERN_C()
192196
char *strtok_r(char *s, const char *delim, char **last);

main/safe_bcmp.c

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/*
2+
+----------------------------------------------------------------------+
3+
| PHP Version 8 |
4+
+----------------------------------------------------------------------+
5+
| Copyright (c) The PHP Group |
6+
+----------------------------------------------------------------------+
7+
| This source file is subject to version 3.01 of the PHP license, |
8+
| that is bundled with this package in the file LICENSE, and is |
9+
| available through the world-wide-web at the following url: |
10+
| http://www.php.net/license/3_01.txt |
11+
| If you did not receive a copy of the PHP license and are unable to |
12+
| obtain it through the world-wide-web, please send a note to |
13+
| [email protected] so we can mail you a copy immediately. |
14+
+----------------------------------------------------------------------+
15+
| Author: |
16+
+----------------------------------------------------------------------+
17+
*/
18+
19+
#include "php.h"
20+
21+
#include <string.h>
22+
23+
PHPAPI int php_safe_bcmp(const void *a, const void *b, size_t siz)
24+
{
25+
const volatile unsigned char *ua = (const volatile unsigned char *)a;
26+
const volatile unsigned char *ub = (const volatile unsigned char *)b;
27+
size_t i = 0;
28+
int r = 0;
29+
30+
while (i < siz) {
31+
r |= ua[i] ^ ub[i];
32+
++i;
33+
}
34+
35+
return r;
36+
}

0 commit comments

Comments
 (0)