From f4f6aaf18a37d6f55b9edead11411c1d05705966 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Tue, 6 Oct 2020 13:29:58 +0200 Subject: [PATCH 1/3] Fix #44618: Fetching may rely on uninitialized data Unless `SQLGetData()` returns `SQL_SUCCESS` or `SQL_SUCCESS_WITH_INFO`, the `StrLen_or_IndPtr` output argument is not guaranteed to be properly set. Thus we handle retrieval failure other than `SQL_ERROR` by silently yielding `false` for those column values. --- ext/odbc/php_odbc.c | 20 ++++++++++--- ext/odbc/tests/bug44618.phpt | 56 ++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 4 deletions(-) create mode 100644 ext/odbc/tests/bug44618.phpt diff --git a/ext/odbc/php_odbc.c b/ext/odbc/php_odbc.c index 1bb832553c52..00b4b15055ba 100644 --- a/ext/odbc/php_odbc.c +++ b/ext/odbc/php_odbc.c @@ -1810,6 +1810,8 @@ static void php_odbc_fetch_hash(INTERNAL_FUNCTION_PARAMETERS, int result_type) if (rc == SQL_SUCCESS_WITH_INFO) { ZVAL_STRINGL(&tmp, buf, result->longreadlen); + } else if (rc != SQL_SUCCESS) { + ZVAL_FALSE(&tmp); } else if (result->values[i].vallen == SQL_NULL_DATA) { ZVAL_NULL(&tmp); break; @@ -1962,6 +1964,8 @@ PHP_FUNCTION(odbc_fetch_into) } if (rc == SQL_SUCCESS_WITH_INFO) { ZVAL_STRINGL(&tmp, buf, result->longreadlen); + } else if (rc != SQL_SUCCESS) { + ZVAL_FALSE(&tmp); } else if (result->values[i].vallen == SQL_NULL_DATA) { ZVAL_NULL(&tmp); break; @@ -2199,12 +2203,12 @@ PHP_FUNCTION(odbc_result) RETURN_FALSE; } - if (result->values[field_ind].vallen == SQL_NULL_DATA) { - zend_string_efree(field_str); - RETURN_NULL(); - } else if (rc == SQL_NO_DATA_FOUND) { + if (rc != SQL_SUCCESS && rc != SQL_SUCCESS_WITH_INFO) { zend_string_efree(field_str); RETURN_FALSE; + } else if (result->values[field_ind].vallen == SQL_NULL_DATA) { + zend_string_efree(field_str); + RETURN_NULL(); } /* Reduce fieldlen by 1 if we have char data. One day we might have binary strings... */ @@ -2250,6 +2254,11 @@ PHP_FUNCTION(odbc_result) RETURN_FALSE; } + if (rc != SQL_SUCCESS && rc != SQL_SUCCESS_WITH_INFO) { + efree(field); + RETURN_FALSE; + } + if (result->values[field_ind].vallen == SQL_NULL_DATA) { efree(field); RETURN_NULL(); @@ -2359,6 +2368,9 @@ PHP_FUNCTION(odbc_result_all) } if (rc == SQL_SUCCESS_WITH_INFO) { PHPWRITE(buf, result->longreadlen); + } else if (rc != SQL_SUCCESS) { + efree(buf); + RETURN_FALSE; } else if (result->values[i].vallen == SQL_NULL_DATA) { php_printf("NULL"); break; diff --git a/ext/odbc/tests/bug44618.phpt b/ext/odbc/tests/bug44618.phpt new file mode 100644 index 000000000000..7aaf4b1557df --- /dev/null +++ b/ext/odbc/tests/bug44618.phpt @@ -0,0 +1,56 @@ +--TEST-- +Bug #44618 (Fetching may rely on uninitialized data) +--SKIPIF-- + +--FILE-- + +--CLEAN-- + +--EXPECT-- +array(3) { + ["ID"]=> + string(1) "1" + ["real1"]=> + string(5) "10.02" + ["text1"]=> + bool(false) +} +array(3) { + [0]=> + string(1) "1" + [1]=> + string(5) "10.02" + [2]=> + bool(false) +} +bool(false) + +
IDreal1text1
110.02 From baf094a4846f4a13e21f313f17f4b399f3da77fe Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Tue, 6 Oct 2020 14:36:49 +0200 Subject: [PATCH 2/3] Close table on failure --- ext/odbc/php_odbc.c | 1 + ext/odbc/tests/bug44618.phpt | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/ext/odbc/php_odbc.c b/ext/odbc/php_odbc.c index 00b4b15055ba..2d690aa5b9a3 100644 --- a/ext/odbc/php_odbc.c +++ b/ext/odbc/php_odbc.c @@ -2369,6 +2369,7 @@ PHP_FUNCTION(odbc_result_all) if (rc == SQL_SUCCESS_WITH_INFO) { PHPWRITE(buf, result->longreadlen); } else if (rc != SQL_SUCCESS) { + php_printf("
"); efree(buf); RETURN_FALSE; } else if (result->values[i].vallen == SQL_NULL_DATA) { diff --git a/ext/odbc/tests/bug44618.phpt b/ext/odbc/tests/bug44618.phpt index 7aaf4b1557df..a573c06f68bb 100644 --- a/ext/odbc/tests/bug44618.phpt +++ b/ext/odbc/tests/bug44618.phpt @@ -53,4 +53,4 @@ array(3) { } bool(false) -
IDreal1text1
110.02 +
110.02
From 2d25d9943ad42f5def4bb022677f5eae25112e1e Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Wed, 28 Oct 2020 14:53:28 +0100 Subject: [PATCH 3/3] Raise warnings instead of silently failing --- ext/odbc/php_odbc.c | 5 +++++ ext/odbc/tests/bug44618.phpt | 8 +++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/ext/odbc/php_odbc.c b/ext/odbc/php_odbc.c index 2d690aa5b9a3..b3ed46d173b5 100644 --- a/ext/odbc/php_odbc.c +++ b/ext/odbc/php_odbc.c @@ -1811,6 +1811,7 @@ static void php_odbc_fetch_hash(INTERNAL_FUNCTION_PARAMETERS, int result_type) if (rc == SQL_SUCCESS_WITH_INFO) { ZVAL_STRINGL(&tmp, buf, result->longreadlen); } else if (rc != SQL_SUCCESS) { + php_error_docref(NULL, E_WARNING, "Can't get data of column #%d (retcode %u)", i + 1, rc); ZVAL_FALSE(&tmp); } else if (result->values[i].vallen == SQL_NULL_DATA) { ZVAL_NULL(&tmp); @@ -1965,6 +1966,7 @@ PHP_FUNCTION(odbc_fetch_into) if (rc == SQL_SUCCESS_WITH_INFO) { ZVAL_STRINGL(&tmp, buf, result->longreadlen); } else if (rc != SQL_SUCCESS) { + php_error_docref(NULL, E_WARNING, "Can't get data of column #%d (retcode %u)", i + 1, rc); ZVAL_FALSE(&tmp); } else if (result->values[i].vallen == SQL_NULL_DATA) { ZVAL_NULL(&tmp); @@ -2205,6 +2207,7 @@ PHP_FUNCTION(odbc_result) if (rc != SQL_SUCCESS && rc != SQL_SUCCESS_WITH_INFO) { zend_string_efree(field_str); + php_error_docref(NULL, E_WARNING, "Can't get data of column #%d (retcode %u)", field_ind + 1, rc); RETURN_FALSE; } else if (result->values[field_ind].vallen == SQL_NULL_DATA) { zend_string_efree(field_str); @@ -2255,6 +2258,7 @@ PHP_FUNCTION(odbc_result) } if (rc != SQL_SUCCESS && rc != SQL_SUCCESS_WITH_INFO) { + php_error_docref(NULL, E_WARNING, "Can't get data of column #%d (retcode %u)", field_ind + 1, rc); efree(field); RETURN_FALSE; } @@ -2370,6 +2374,7 @@ PHP_FUNCTION(odbc_result_all) PHPWRITE(buf, result->longreadlen); } else if (rc != SQL_SUCCESS) { php_printf(""); + php_error_docref(NULL, E_WARNING, "Can't get data of column #%d (retcode %u)", i + 1, rc); efree(buf); RETURN_FALSE; } else if (result->values[i].vallen == SQL_NULL_DATA) { diff --git a/ext/odbc/tests/bug44618.phpt b/ext/odbc/tests/bug44618.phpt index a573c06f68bb..aed713cdfbb0 100644 --- a/ext/odbc/tests/bug44618.phpt +++ b/ext/odbc/tests/bug44618.phpt @@ -34,7 +34,8 @@ include __DIR__ . "/config.inc"; $conn = odbc_connect($dsn, $user, $pass); odbc_exec($conn, "DROP TABLE bug44618"); ?> ---EXPECT-- +--EXPECTF-- +Warning: odbc_fetch_array(): Can't get data of column #3 (retcode 100) in %s on line %d array(3) { ["ID"]=> string(1) "1" @@ -43,6 +44,8 @@ array(3) { ["text1"]=> bool(false) } + +Warning: odbc_fetch_into(): Can't get data of column #3 (retcode 100) in %s on line %d array(3) { [0]=> string(1) "1" @@ -51,6 +54,9 @@ array(3) { [2]=> bool(false) } + +Warning: odbc_result(): Can't get data of column #3 (retcode 100) in %s on line %d bool(false)
IDreal1text1
110.02
+Warning: odbc_result_all(): Can't get data of column #3 (retcode 100) in %s on line %d