Skip to content

php_odbc_fetch_hash() Fixed a segfault when fetching certain SQL NULLs #193

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
Closed

php_odbc_fetch_hash() Fixed a segfault when fetching certain SQL NULLs #193

Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d0d978b
php_odbc_fetch_hash() Fixed a segfault when fetching certain SQL NULLs
Sep 13, 2012
1a72814
Merge branch 'PHP-5.3' into PHP-5.4
Sep 13, 2012
1f11dde
Merge branch 'PHP-5.4'
Sep 13, 2012
383fcb3
php_odbc_fetch_hash() Fixed a segfault when fetching certain SQL NULLs
Sep 13, 2012
1f55a42
Merge branch 'PHP-5.3'
Sep 13, 2012
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ext/odbc/php_odbc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1765,7 +1765,7 @@ static void php_odbc_fetch_hash(INTERNAL_FUNCTION_PARAMETERS, int result_type)
if (result_type & ODBC_NUM) {
zend_hash_index_update(Z_ARRVAL_P(return_value), i, &tmp, sizeof(zval *), NULL);
} else {
if (!*(result->values[i].name)) {
if (!*(result->values[i].name) && Z_TYPE_P(tmp) != IS_NULL) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be Z_TYPE_P(tmp) == IS_STRING, rather than not IS_NULL? From context, it's obvious that the only two possible zval types for tmp are IS_STRING and IS_NULL, but this strikes me as a potential gotcha if the function is changed down the track to return more native PHP types for numeric values.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adam I think you are right about that. Changing to Z_TYPE_P(tmp) ==
IS_STRING seems like a better way to avoid the problem.

I will ask for help on IRC for how to submit a better fix.

Thanks,
Brandon Kirsch

On Wed, Sep 12, 2012 at 11:13 PM, Adam Harvey notifications@github.comwrote:

In ext/odbc/php_odbc.c:

@@ -1765,7 +1765,7 @@ static void php_odbc_fetch_hash(INTERNAL_FUNCTION_PARAMETERS, int result_type)
if (result_type & ODBC_NUM) {
zend_hash_index_update(Z_ARRVAL_P(return_value), i, &tmp, sizeof(zval *), NULL);
} else {

  •       if (!*(result->values[i].name)) {

  •       if (!*(result->values[i].name) && Z_TYPE_P(tmp) != IS_NULL) {

Should this be Z_TYPE_P(tmp) == IS_STRING, rather than not IS_NULL? From
context, it's obvious that the only two possible zval types for tmp are
IS_STRING and IS_NULL, but this strikes me as a potential gotcha if the
function is changed down the track to return more native PHP types for
numeric values.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/193/files#r1594144.

zend_hash_update(Z_ARRVAL_P(return_value), Z_STRVAL_P(tmp), Z_STRLEN_P(tmp)+1, &tmp, sizeof(zval *), NULL);
} else {
zend_hash_update(Z_ARRVAL_P(return_value), result->values[i].name, strlen(result->values[i].name)+1, &tmp, sizeof(zval *), NULL);
Expand Down