Handle memory limit error during string reallocation correctly
Do not decrement the refcount before allocating the new string,
as the allocation operation may bail out and cause a use-after-free
lateron. We can only decrement the refcount once the allocation
has succeeded.
Fix throwing of yield from related exceptions into generator
Use the general zend_generator_throw_exception() helper for this.
Otherwise we don't handle the off-by-one opline correctly (should
we maybe just stop doing that?)
This is a followup to ad750c3bb6e7b48384c6265eb9d3bcf5b4000652,
which fixed a different yield from exception handling problem that
happened to show up in the same test case from oss-fuzz #25321.
Now both issues should be fixed.
Matteo Beccati [Mon, 31 Aug 2020 08:45:36 +0000 (10:45 +0200)]
Fix #80027 Terrible performance using $query->fetch on queries with many bind parameters
Added new flags that allow skipping param_evt(s) that are not used by drivers,
in a backwards and forward compatible manner. Updated the pgsql, mysql, sqlite
and oci drivers to properly use the new flags. I've left out pdo_dblib, which
doesn't have a param_hook, and pdo_firebird, which seems to be using
PARAM_EVT_NORMALIZE in a wrong context (param type vs event type).
Nikita Popov [Wed, 12 Aug 2020 08:09:37 +0000 (10:09 +0200)]
Make MAX_IFD_NESTING_LEVEL an actual nesting level
Currently we only ever increment ifd_nesting_level, so this ends up
being a limit on the total number of IFD tags and we regularly get
bug reports of it being exceeded. I think the intention behind this
limit was to prevent recursion stack overflow, and for that we only
need to check actual recursive usage. I've implemented that here,
and dropped the nesting limit down to a smaller value
(which still passes our tests).
However, it seems that we do also need to have a total limit on
the number of tags, as we don't catch some instances of infinite
looping otherwise. Add this as a separate limit with a higher
value, that should hopefully be sufficient.
Nikita Popov [Thu, 27 Aug 2020 13:49:33 +0000 (15:49 +0200)]
Require non-negative length in stream_get_contents()
If the length is not -1, require it to be non-negative.
Using such lengths doesn't make sense (as only -1 is special-case
to read in chunks, anything else will end up doing a huge upfront
allocation) and can lead to string allocation overflow.
A similar check is already in place for file_get_contents(). That
one does not allow -1 (and uses null instead), but this function
is explicitly specified to accept -1, so stick to that behavior.
Fix #64130: COM obj parameters passed by reference are not updated
`ITypeInfo_GetIDsOfNames()` is supposed to fail with `E_NOTIMPL` for
out-of-process servers, thus we should not remove the already available
typeinfo of the object in this case.
Fix #79986: str_ireplace bug with diacritics characters
`tolower()` returns an `int`, so we must not convert to `char` which
may be `signed` and as such may be subject to overflow (actually,
implementation defined behavior).
Ahmed Abdou [Sun, 17 Feb 2019 21:59:00 +0000 (22:59 +0100)]
Fix #64705 errorInfo property of PDOException is null when PDO::__construct() fails
PDO driver constructors are throwing PdoException without setting
errorInfo, so create a new reusable function that throws exceptions
for PDO and will also set the errorInfo. Use this function in
pdo_mysql, pdo_sqlite, and pdo_pgsql.
Fix bug #75785 by attempt switching endianness on Maker's Note
Different manufacturer models may come with a
different endianness (motorola/intel) format. In
order to avoid a big refactor and a gigantic lookup
table, this commit simply attempts to switch the
endianness and proceed when values are acceptable.
Fix #48585: com_load_typelib holds reference, fails on second call
Whether the type library is cached is actually irrelevant here; what
matters is that the symbols are imported, and since these are not
cached, we have to import them for every request. And we cannot cache
the symbols, because the import depends on the current codepage, but
the codepage is a `PHP_INI_ALL` setting.
Fix #79922: Crash after multiple calls to xml_parser_free()
We must not call `zend_list_delete()` in resource closer functions
exposed to userland, because decreasing the refcount there leads to
use-after-free scenarios. In this case, commit 4a42fbb worked for
typical use-cases where `xml_parser_free()` has been called exactly
once for the resource, because there is an internal zval (`->index`)
referencing the same resource which already increased the refcount by
one. However, when `xml_parser_free()` is called multiple times on the
same XML parser resource, the resource would be freed prematurely.
Instead we forcefully close the resource in `xml_parser_free()`. We
also could decrease the refcount of the resource there, but that would
require to call `xml_parser_free()` which is somewhat uncommon, and
would be particularly bad wrt. PHP 8 where that function is a NOP, and
as such doesn't have to be called. So we do no longer increase the
refcount of the resource when copying it to the internal zval, and let
the usualy refcounting semantics take care of the resource destruction.
Fix #73060: php failed with error after temp folder cleaned up
Instead of storing the mapping base address and the address of
`execute_ex()` in a separate file in the temporary folder, we store
them right at the beginning of the memory mapping.
The file extension to mime type mapping *must* not depend on the file
extension's case for case-insensitive file systems, and *should* not
for case-sensitive file systems.
It does not make sense to make assumptions about `PHP_CONFIG_FILE_PATH`
during build time, since that value is never used during run time on
Windows. Since there is no `--with-config-file-path` on Windows
either, we define `PHP_CONFIG_FILE_PATH` as `""`.
This test fails occasionally due to timing issues, because the session
file may have been unlinked by the first `session_start()`'s GC. We
adapt the test expectation to this reality.
Fix #63527: DCOM does not work with Username, Password parameter
We must not mix multibyte and wide character strings in the
`COAUTHIDENTITY` structure. Using wide character strings throughout
would have the advantage that the remote connection can be established
regardless of the code page of the server, but that would more likely
break BC, so we just drop the wide character string conversion of the
username.
In the interest of avoiding side-effects during dumping, I'm
replacing the value with a <constant ast> string instead of
performing an update constant operation.
Nikita Popov [Tue, 30 Jun 2020 15:28:47 +0000 (17:28 +0200)]
Remove bogus generator iterator dtor
Fixes a use-after-free encountered in Symfony's SecurityBundle.
I don't have a reproducer for this, and believe the issue can only
occur if we leak an iterator (the leak is a separate issue).
We should not free the generator iterator here, because we do not
own it. The code that fetched the iterator is responsible for
releasing it. In the rare case where we do hit this code-path,
we cause a use-after-free.
Fix #63208: BSTR to PHP string conversion not binary safe
A `BSTR` is similar to a `zend_string`; it stores the length of the
string just before the actual string, and thus the string may contain
NUL bytes. However, `php_com_olestring_to_string()` is supposed to
deal with arbitrary `OLECHAR*`s which may not be `BSTR`s, so we
introduce `php_com_bstr_to_string()` and use it for the only case where
we actually have to deal with `BSTR`s which may contain NUL bytes.
Contrary to `php_com_olestring_to_string()` we return a `zend_string`,
so we can save the re-allocation when converting to a `zval`.
We also cater to `php_com_string_to_olestring()` not being binary safe,
with basically the same fix we did for `php_com_olestring_to_string()`.
`atol()` returns a `long` which is not the same as `zend_long` on
LLP64; we use `ZEND_ATOL()` instead.
There is no need for a new test case, since filesize_large.phpt already
tests for that behavior; unfortunately, the FTP test suite relies on
`pcntl_fork()` and therefore cannot be run on Windows.
Even if the length of a maker note does not match our expectations
(either because the maker note is corrupted, or because our
expectations do not quite match reality), there is no need to let
parsing fail; we can still go on parsing the other meta information.
The `timercmp()` manpage[1] points out that some systems have a broken
implementation which does not support `>=`. This is definitely the
case for the Windows SDK, which only supports `<` and `>`.
projects / php / log