Repository navigation
Conversation
Regressed in 6fbf81c. There is a missing error check on spl_filesystem_file_read_line(), which means that if the line could not be read (e.g. because we're at the end of the file), it will not set intern->u.file.current_line, which will cause a NULL pointer deref later on. Fix it by adding a check, and reintroducing the silent flag partially to be able to throw an exception like it did in the past.
| if (!intern->u.file.current_line && Z_ISUNDEF(intern->u.file.current_zval)) { | ||
| spl_filesystem_file_read_line(ZEND_THIS, intern); | ||
| if (!intern->u.file.current_line) { | ||
| ZEND_ASSERT(Z_ISUNDEF(intern->u.file.current_zval)); |
There was a problem hiding this comment.
One thing that confused me is the original check: !intern->u.file.current_line && Z_ISUNDEF(intern->u.file.current_zval). I think that if one of these conditions in the && is false, the other one is as well.
If that is not the case then it will still be buggy (in other places too) because there could be a potential NULL deref if it is possible that Z_ISUNDEF(intern->u.file.current_zval) is false while !intern->u.file.current_line is true.
That's why I changed it to an assert.
There was a problem hiding this comment.
Isn't it possible if one does some rewind or something like that? But yeah that does seem lightly weird otherwise
There was a problem hiding this comment.
Hm I don't think so, rewind calls spl_filesystem_file_free_line which (confusingly) also clears out current_zval...
Regressed in 6fbf81c.
There is a missing error check on spl_filesystem_file_read_line(), which means that if the line could not be read (e.g. because we're at the end of the file), it will not set intern->u.file.current_line, which will cause a NULL pointer deref later on.
Fix it by adding a check, and reintroducing the silent flag partially to be able to throw an exception like it did in the past.