Copy parameters before converting them to strings

[SakiTakamachi]

https://github.com/php/php-src/actions/runs/8119296548/job/22195005842

convert_to_string was changing parameter directly, and the libmysql test told me that.

[nielsdos]

Note that this will leak memory now.
Aside, a better approach would be to use zval_get_string on parameter.
That being said, you can't release the memory after setting the buffer because that would cause a use-after-free... so you do have to store and release the strings somewhere...

[SakiTakamachi]

Thanks, after opening the PR, I felt like it was going to leak, so I was just looking into ZVAL_COPY.

I haven't figured out enought the macros for zval yet, so your comment was helpful.

I'll try your suggestion tomorrow.

[SakiTakamachi]

I checked other drivers, and found that some used convert_to_string() in similar situations.

Rather, maybe I should change the behavior when using mysqlnd.

However, I'm not sure if it's correct to change the user's variables for PDO's convenience, given the behavior of PDO as a whole....

[nielsdos]

I don't know, it feels strange to convert it in place, but I don't immediately see another simple solution.
cc @kamil-tekiela.

[kamil-tekiela]

Do we have to convert it into a string? Is there no other way of getting the value from a stringable object?

[nielsdos]

Just tested it, I can reproduce the leak with the following script (note: it's also necessary to disable emulated prepares to trigger this):

<?php

class Test
{
    public function __toString(): string
    {
        return str_repeat('555', random_int(1, 1));
    }
}

$db = new PDO("mysql:host=127.0.0.1", "root", "1234");
$db->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);

$stmt = $db->prepare('SELECT ?');
$param = 'foo';
$stmt->bindParam(1, $param, PDO::PARAM_STR);
$param = new Test();
$stmt->execute();

irect leak of 35 byte(s) in 1 object(s) allocated from:
    #0 0x7f0c0cee1359 in __interceptor_malloc /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 0x5bb0bf5b43f6 in __zend_malloc /run/media/niels/MoreData/php-src/Zend/zend_alloc.c:3081
    #2 0x5bb0bf5b1c4e in _emalloc /run/media/niels/MoreData/php-src/Zend/zend_alloc.c:2580
    #3 0x5bb0bf5b1fe0 in _safe_emalloc /run/media/niels/MoreData/php-src/Zend/zend_alloc.c:2624
    #4 0x5bb0bf2b12fa in zend_string_safe_alloc /run/media/niels/MoreData/php-src/Zend/zend_string.h:187
    #5 0x5bb0bf2ed276 in zif_str_repeat /run/media/niels/MoreData/php-src/ext/standard/string.c:5501
    #6 0x5bb0bf6fea44 in ZEND_DO_ICALL_SPEC_RETVAL_USED_HANDLER /run/media/niels/MoreData/php-src/Zend/zend_vm_execute.h:1349
    #7 0x5bb0bf858fc9 in execute_ex /run/media/niels/MoreData/php-src/Zend/zend_vm_execute.h:57350
    #8 0x5bb0bf61677f in zend_call_function /run/media/niels/MoreData/php-src/Zend/zend_execute_API.c:971
    #9 0x5bb0bf61779c in zend_call_known_function /run/media/niels/MoreData/php-src/Zend/zend_execute_API.c:1065
    #10 0x5bb0bf8e34c8 in zend_call_known_instance_method /run/media/niels/MoreData/php-src/Zend/zend_API.h:857
    #11 0x5bb0bf8e3502 in zend_call_known_instance_method_with_0_params /run/media/niels/MoreData/php-src/Zend/zend_API.h:863
    #12 0x5bb0bf8f1bb7 in zend_std_cast_object_tostring /run/media/niels/MoreData/php-src/Zend/zend_object_handlers.c:1934
    #13 0x5bb0bf63162c in __zval_get_string_func /run/media/niels/MoreData/php-src/Zend/zend_operators.c:1034
    #14 0x5bb0bf631883 in zval_get_string_func /run/media/niels/MoreData/php-src/Zend/zend_operators.c:1055
    #15 0x5bb0bef4dfaf in zval_get_string /run/media/niels/MoreData/php-src/Zend/zend_operators.h:314
    #16 0x5bb0bef51663 in pdo_mysql_stmt_param_hook /run/media/niels/MoreData/php-src/ext/pdo_mysql/mysql_statement.c:553
    #17 0x5bb0bef2c440 in dispatch_param_event /run/media/niels/MoreData/php-src/ext/pdo/pdo_stmt.c:112
    #18 0x5bb0bef2f0cd in zim_PDOStatement_execute /run/media/niels/MoreData/php-src/ext/pdo/pdo_stmt.c:458
    #19 0x5bb0bf70252d in ZEND_DO_FCALL_SPEC_RETVAL_UNUSED_HANDLER /run/media/niels/MoreData/php-src/Zend/zend_vm_execute.h:1879
    #20 0x5bb0bf859219 in execute_ex /run/media/niels/MoreData/php-src/Zend/zend_vm_execute.h:57390
    #21 0x5bb0bf86ce20 in zend_execute /run/media/niels/MoreData/php-src/Zend/zend_vm_execute.h:62752
    #22 0x5bb0bf657811 in zend_execute_script /run/media/niels/MoreData/php-src/Zend/zend.c:1888
    #23 0x5bb0bf4d3137 in php_execute_script_ex /run/media/niels/MoreData/php-src/main/main.c:2507
    #24 0x5bb0bf4d353f in php_execute_script /run/media/niels/MoreData/php-src/main/main.c:2547
    #25 0x5bb0bfa4f3ef in do_cli /run/media/niels/MoreData/php-src/sapi/cli/php_cli.c:966
    #26 0x5bb0bfa511bc in main /run/media/niels/MoreData/php-src/sapi/cli/php_cli.c:1340
    #27 0x7f0c0c974ccf  (/usr/lib/libc.so.6+0x25ccf) (BuildId: c0caa0b7709d3369ee575fcd7d7d0b0fc48733af)

[nielsdos]

So we do have to store the temporary string somewhere and release it later.
I wonder what the extension field in PDO_MYSQL_PARAM_BIND is used for.
I know these kinds of "extension" fields exist sometimes in libraries so that library users can attach extra data. Whether that's the intended purpose here I don't know.
If it is, perhaps we could store a pointer to the temporary string in there so we can release it later. But this is just a rough quick idea instead of something concrete. I could be completely wrong about the purpose of that field.
Or shove it somewhere in pdo_bound_param_data if that's not an option. But there's a chance there's a much better idea than these two.

[kamil-tekiela]

For the time being, maybe we can revert this change for libmysql. I think it works fine with mysqlnd.

[SakiTakamachi]

@nielsdos
Amazing, thanks!

[SakiTakamachi]

@kamil-tekiela
I agree to revert. I need more time.

[SakiTakamachi]

I reverted it.
3dc9503