Skip to content

Commit e437b4b

Browse files
committed
Zend/zend_string: add zend_string_starts_with()
This adds missing length checks to several callers, e.g. in cache_script_in_shared_memory(). This is important when the zend_string is shorter than the string parameter, when memcmp() happens to check backwards; this can result in an out-of-bounds memory access.
1 parent 329d668 commit e437b4b

File tree

6 files changed

+19
-12
lines changed

6 files changed

+19
-12
lines changed

Zend/zend_string.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,19 @@ static zend_always_inline bool zend_string_equals(zend_string *s1, zend_string *
379379
#define zend_string_equals_literal(str, literal) \
380380
zend_string_equals_str(str, literal, sizeof(literal) - 1)
381381

382+
static zend_always_inline bool zend_string_starts_with_str(const zend_string *str, const char *prefix, size_t prefix_length)
383+
{
384+
return ZSTR_LEN(str) >= prefix_length && !memcmp(ZSTR_VAL(str), prefix, prefix_length);
385+
}
386+
387+
static zend_always_inline bool zend_string_starts_with_val(const zend_string *str, const zend_string *prefix)
388+
{
389+
return zend_string_starts_with_str(str, ZSTR_VAL(prefix), ZSTR_LEN(prefix));
390+
}
391+
392+
#define zend_string_starts_with_literal(str, prefix) \
393+
zend_string_starts_with_str(str, prefix, sizeof(prefix) - 1)
394+
382395
/*
383396
* DJBX33A (Daniel J. Bernstein, Times 33 with Addition)
384397
*

ext/opcache/ZendAccelerator.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1616,7 +1616,7 @@ static zend_persistent_script *cache_script_in_shared_memory(zend_persistent_scr
16161616
zend_accel_error(ACCEL_LOG_INFO, "Cached script '%s'", ZSTR_VAL(new_persistent_script->script.filename));
16171617
if (key &&
16181618
/* key may contain non-persistent PHAR aliases (see issues #115 and #149) */
1619-
memcmp(ZSTR_VAL(key), "phar://", sizeof("phar://") - 1) != 0 &&
1619+
!zend_string_starts_with_literal(key, "phar://") &&
16201620
!zend_string_equals(new_persistent_script->script.filename, key)) {
16211621
/* link key to the same persistent script in hash table */
16221622
zend_string *new_key = accel_new_interned_key(key);

ext/phar/stream.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -912,8 +912,7 @@ static int phar_wrapper_rename(php_stream_wrapper *wrapper, const char *url_from
912912

913913
ZEND_HASH_MAP_FOREACH_BUCKET(&phar->virtual_dirs, b) {
914914
str_key = b->key;
915-
if (ZSTR_LEN(str_key) >= from_len &&
916-
memcmp(ZSTR_VAL(str_key), ZSTR_VAL(resource_from->path)+1, from_len) == 0 &&
915+
if (zend_string_starts_with_str(str_key, ZSTR_VAL(resource_from->path)+1, from_len) &&
917916
(ZSTR_LEN(str_key) == from_len || IS_SLASH(ZSTR_VAL(str_key)[from_len]))) {
918917

919918
new_str_key = zend_string_alloc(ZSTR_LEN(str_key) + to_len - from_len, 0);
@@ -930,8 +929,7 @@ static int phar_wrapper_rename(php_stream_wrapper *wrapper, const char *url_from
930929

931930
ZEND_HASH_MAP_FOREACH_BUCKET(&phar->mounted_dirs, b) {
932931
str_key = b->key;
933-
if (ZSTR_LEN(str_key) >= from_len &&
934-
memcmp(ZSTR_VAL(str_key), ZSTR_VAL(resource_from->path)+1, from_len) == 0 &&
932+
if (zend_string_starts_with_str(str_key, ZSTR_VAL(resource_from->path)+1, from_len) &&
935933
(ZSTR_LEN(str_key) == from_len || IS_SLASH(ZSTR_VAL(str_key)[from_len]))) {
936934

937935
new_str_key = zend_string_alloc(ZSTR_LEN(str_key) + to_len - from_len, 0);

ext/spl/spl_directory.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -740,7 +740,7 @@ void spl_filesystem_object_construct(INTERNAL_FUNCTION_PARAMETERS, zend_long cto
740740
/* spl_filesystem_dir_open() may emit an E_WARNING */
741741
zend_replace_error_handling(EH_THROW, spl_ce_UnexpectedValueException, &error_handling);
742742
#ifdef HAVE_GLOB
743-
if (SPL_HAS_FLAG(ctor_flags, DIT_CTOR_GLOB) && memcmp(ZSTR_VAL(path), "glob://", sizeof("glob://")-1) != 0) {
743+
if (SPL_HAS_FLAG(ctor_flags, DIT_CTOR_GLOB) && !zend_string_starts_with_literal(path, "glob://")) {
744744
path = zend_strpprintf(0, "glob://%s", ZSTR_VAL(path));
745745
spl_filesystem_dir_open(intern, path);
746746
zend_string_release(path);

ext/standard/proc_open.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -728,7 +728,7 @@ static zend_result set_proc_descriptor_to_pipe(descriptorspec_item *desc, zend_s
728728

729729
desc->type = DESCRIPTOR_TYPE_PIPE;
730730

731-
if (strncmp(ZSTR_VAL(zmode), "w", 1) != 0) {
731+
if (!zend_string_starts_with_literal(zmode, "w")) {
732732
desc->parentend = newpipe[1];
733733
desc->childend = newpipe[0];
734734
desc->mode_flags = O_WRONLY;

ext/standard/string.c

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1810,11 +1810,7 @@ PHP_FUNCTION(str_starts_with)
18101810
Z_PARAM_STR(needle)
18111811
ZEND_PARSE_PARAMETERS_END();
18121812

1813-
if (ZSTR_LEN(needle) > ZSTR_LEN(haystack)) {
1814-
RETURN_FALSE;
1815-
}
1816-
1817-
RETURN_BOOL(memcmp(ZSTR_VAL(haystack), ZSTR_VAL(needle), ZSTR_LEN(needle)) == 0);
1813+
RETURN_BOOL(zend_string_starts_with_val(haystack, needle));
18181814
}
18191815
/* }}} */
18201816

0 commit comments

Comments
 (0)