From 2aa6bdb4249910306642e070a3778ba202e73c91 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Sat, 12 Dec 2020 13:40:08 +0100 Subject: [PATCH 1/3] Disallow version_compare() $operator abbreviations `version_compare()` does a sloppy check for the `$operators` argument which allows arbitrary abbreviations of the supported operators to be accepted. This is both undocumented and unexpected, and could lead to subtle BC breaks, if the order of the comparisions will be changed. Therefore we change to strict comparisons. --- ext/standard/versioning.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/ext/standard/versioning.c b/ext/standard/versioning.c index dbdae6ecf7532..8df4db601b229 100644 --- a/ext/standard/versioning.c +++ b/ext/standard/versioning.c @@ -203,7 +203,8 @@ php_version_compare(const char *orig_ver1, const char *orig_ver2) PHP_FUNCTION(version_compare) { - char *v1, *v2, *op = NULL; + char *v1, *v2; + zend_string *op = NULL; size_t v1_len, v2_len, op_len = 0; int compare; @@ -211,29 +212,29 @@ PHP_FUNCTION(version_compare) Z_PARAM_STRING(v1, v1_len) Z_PARAM_STRING(v2, v2_len) Z_PARAM_OPTIONAL - Z_PARAM_STRING_OR_NULL(op, op_len) + Z_PARAM_STR_OR_NULL(op) ZEND_PARSE_PARAMETERS_END(); compare = php_version_compare(v1, v2); if (!op) { RETURN_LONG(compare); } - if (!strncmp(op, "<", op_len) || !strncmp(op, "lt", op_len)) { + if (zend_string_equals_literal(op, "<") || zend_string_equals_literal(op, "lt")) { RETURN_BOOL(compare == -1); } - if (!strncmp(op, "<=", op_len) || !strncmp(op, "le", op_len)) { + if (zend_string_equals_literal(op, "<=") || zend_string_equals_literal(op, "le")) { RETURN_BOOL(compare != 1); } - if (!strncmp(op, ">", op_len) || !strncmp(op, "gt", op_len)) { + if (zend_string_equals_literal(op, ">") || zend_string_equals_literal(op, "gt")) { RETURN_BOOL(compare == 1); } - if (!strncmp(op, ">=", op_len) || !strncmp(op, "ge", op_len)) { + if (zend_string_equals_literal(op, ">=") || zend_string_equals_literal(op, "ge")) { RETURN_BOOL(compare != -1); } - if (!strncmp(op, "==", op_len) || !strncmp(op, "=", op_len) || !strncmp(op, "eq", op_len)) { + if (zend_string_equals_literal(op, "==") || zend_string_equals_literal(op, "=") || zend_string_equals_literal(op, "eq")) { RETURN_BOOL(compare == 0); } - if (!strncmp(op, "!=", op_len) || !strncmp(op, "<>", op_len) || !strncmp(op, "ne", op_len)) { + if (zend_string_equals_literal(op, "!=") || zend_string_equals_literal(op, "<>") || zend_string_equals_literal(op, "ne")) { RETURN_BOOL(compare != 0); } From 78f817d31ea1a5683b79663551f850e9c5af5ba7 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Sat, 12 Dec 2020 14:26:16 +0100 Subject: [PATCH 2/3] Remove unused variable --- ext/standard/versioning.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/standard/versioning.c b/ext/standard/versioning.c index 8df4db601b229..f4f20c9850a0d 100644 --- a/ext/standard/versioning.c +++ b/ext/standard/versioning.c @@ -205,7 +205,7 @@ PHP_FUNCTION(version_compare) { char *v1, *v2; zend_string *op = NULL; - size_t v1_len, v2_len, op_len = 0; + size_t v1_len, v2_len; int compare; ZEND_PARSE_PARAMETERS_START(2, 3) From c847d84bd501880a1d9a0e58a02cd7eebe0f65c8 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Sun, 13 Dec 2020 19:00:51 +0100 Subject: [PATCH 3/3] Add test to check all formerly supported operator abbreviations --- .../versioning/version_compare_op_abbrev.phpt | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 ext/standard/tests/versioning/version_compare_op_abbrev.phpt diff --git a/ext/standard/tests/versioning/version_compare_op_abbrev.phpt b/ext/standard/tests/versioning/version_compare_op_abbrev.phpt new file mode 100644 index 0000000000000..241c1559feb33 --- /dev/null +++ b/ext/standard/tests/versioning/version_compare_op_abbrev.phpt @@ -0,0 +1,21 @@ +--TEST-- +version_compare() no longer supports operator abbreviations +--FILE-- + +--EXPECT-- +'' failed +'l' failed +'g' failed +'e' failed +'!' failed +'n' failed