After Nikita Popov found a buffer overrun bug in one of my pull
requests, I was prompted to add more assertions in a38c7e5703 to help
me catch such bugs myself more easily in testing.
Wouldn't you just know it... as soon as I added those assertions, the
mbstring test suite caught another buffer overrun bug in my UTF-7
conversion code, which I wrote the better part of a year ago.
Then, when I started fuzzing the code with libfuzzer, I found
and fixed another buffer overflow:
If we enter the main loop, which normally outputs 3 decoded Base64
characters, where the first half of a surrogate pair had appeared at
the end of the previous run, but the second half does not appear
on this run, we need to output one error marker.
Then, at the end of the main loop, if the Base64 input ends at an
unexpected position AND the last character was not a legal
Base64-encoded character, we need to output two error markers
for that. The three error markers plus two valid, decoded bytes
can push us over the available space in our wchar buffer.
When testing the preceding commits, I used a script to generate a large
number of random strings and try to find strings which would yield
different outputs from the new and old encoding conversion code.
Some were found. In most cases, analysis revealed that the new code
was correct and the old code was not.
In all cases where the new code was incorrect, regression tests were
added. However, there may be some value in adding regression tests
for cases where the old code was incorrect as well. That is done here.
This does not cover every case where the new and old code yielded
different results. Some of them were very obscure, and it is proving
difficult even to reproduce them (since I did not keep a record of
all the input strings which triggered the differing output).
One bug in the previous implementation; when it saw a sequence of
codepoints which looked like they might need to be emitted as a special
KDDI emoji, it would totally forget whether it was in ASCII mode,
JISX 0208 mode, or something else. So it could not reliably emit the
correct escape sequence to switch to the right mode.
Further, if the input ends with a codepoint which looks like it could
be part of a special KDDI emoji, then the legacy code did not emit
an escape sequence to switch back to ASCII mode at the end of the
string. This means that the emitted ISO-2022-JP-KDDI strings could not
always be safely concatenated.
There were bugs in the legacy implementation. Lots of them.
It did not properly track whether it has switched to JISX 0213 plane 1
or plane 2. If it processes a character in plane 1 and then immediately
one in plane 2, it failed to emit the escape code to switch to plane 2.
Further, when converting codepoints from 0x80-0xFF to ISO-2022-JP-2004,
the legacy implementation would totally disregard which mode it was
operating in. Such codepoints would pass through directly to the output
without any escape sequences being emitted.
If that was not enough, all the legacy implementations of JISX 0213:2004
encodings had another common bug; their 'flush function' did not call
the next flush function in the chain of conversion filters. So if any
of these encodings were converted to an encoding where the flush
function was needed to finish the output string, then the output
would be truncated.
All the legacy implementations of JISX 0213:2004 encodings had a
common bug; their 'flush function' did not call the next flush function
in the chain of conversion filters. So if any of these encodings were
converted to an encoding where the flush function was needed to finish
the output string, then the output would be truncated.
All the legacy implementations of JISX 0213:2004 encodings had a
common bug; their 'flush function' did not call the next flush function
in the chain of conversion filters. So if any of these encodings were
converted to an encoding where the flush function was needed to finish
the output string, then the output would be truncated.
When working on this, I read RFC 1557 again and realized that the
comment at the top of the file was totally mistaken. Further, the
legacy code did not obey the RFC. (It would emit the "ESC $ ) C"
sequence anywhere, not just at the beginning of a line as the RFC
requires.)
The new code obeys the RFC; one quirk is that it always emits the
escape sequence at the beginning of each output string, even if the
string is completely ASCII (in which case the escape sequence is
allowed, but not required).
The new code doesn't always generate the same number of error markers
for invalid escapes as the old code did.
The old code could not emit the special KDDI emoji for national flags.
Further, there was a bug in the test which the old code used to
determine whether an 0xF byte should be emitted at the end of a string
(to switch back to ASCII mode). As a result, it would not always switch
back to ASCII mode, meaning that it was not always safe to concatenate
the resulting strings.
when passing an int to a string enum. Previously, the int was coerced to
a string. The JIT skips parameter clean up when unnecessary. In this
particular case, passing int to from(int|string) normally doesn't cause
a coercion so no dtor for the $value zval is generated.
To circumvent this we avoid coersion by explicitly allowing ints and
converting them to strings ourselves. Then we can free it appropriately.
See GH-8518
Closes GH-8633
Because the UID= and PWD= values are appended to the SQLDriverConnect
case when credentials are passed, we have to append them to the string
in case users are relying on this behaviour. However, they must be
quoted, or the arguments will be invalid (or possibly more injected).
This means users had to quote arguments or append credentials to the raw
connection string themselves.
It seems that ODBC quoting rules are consistent enough (and that
Microsoft trusts them enough to encode into the .NET BCL) that we can
actually check if the string is already quoted (in case a user is
already quoting because of this not being fixed), and if not, apply the
appropriate ODBC quoting rules.
This is because the code exists in main/, and are shared between
both ODBC extensions, so it doesn't make sense for it to only exist
in one or the other. There may be a better spot for it.
Closes GH-8307.
Kamil Tekiela