ext/session: Fix NULL pointer dereference in SessionHandler::create_sid() - #22580
ext/session: Fix NULL pointer dereference in SessionHandler::create_sid()#22580jorgsowa wants to merge 1 commit into
Conversation
jorgsowa
changed the title
|
Would you like to check this PR @iliaal ? |
|
Looks right, might be worth retarget to 8.4 branch, since the issue is there too. This and #22924 cover the last two of the six s_create_sid() call sites; the other four already check. |
bbee2d7 to
304f37e
Compare
|
Re-targeted to 8.4 |
|
@TimWolla can |
Depends on your definition of "in practice": Not on modern operating systems / kernels. But if the Since we don't currently require a working CSPRNG, this should be treated as a bug fix and backported as appropriate. |
304f37e to
740786f
Compare
|
You can merge this PR @jorgsowa, not sure what is holding you back? |
|
Lack of time recently, sorry. I will do it today. Please, don't treat rebases as pushing you, because I do always to ensure I don't push anything breaking to master. |
s_create_sid() can return NULL when php_random_bytes_throw() fails (e.g. CSPRNG exhaustion), but RETURN_STR() dereferences the string unconditionally. Every other internal caller of s_create_sid() in session.c (php_session_initialize, session_regenerate_id) already NULL-checks the result; this PHP-facing method, reachable from any userland SessionHandler subclass via create_sid(), did not. No dedicated regression test is added: forcing php_random_bytes_throw() to fail is not portably reproducible from a .phpt test (it's a raw getrandom() syscall on Linux and CCRandomGenerateBytes on macOS, neither of which can be faulted from userland), which is also why the existing NULL-checks this mirrors in session.c have none either.
740786f to
5e1f8f4
Compare
|
Closed by d974c64 |
4 participants
s_create_sid()can return NULL whenphp_random_bytes_throw()fails (e.g. CSPRNG exhaustion). Every other internal caller ofs_create_sid()in session.c (php_session_initialize, session_regenerate_id) already NULL-checks the result.It's not possible to test it through .phpt test file. Should it be pointed to the PHP8.4 branch?