Hi all,
If no one objects, I plan on merging
https://github.com/willfitch/php-src/commit/06170d344f6b3148d505afd8ae952d3439de9005
to 5.3 and up later tonight. This bug has been out there for a long
time.
Will
Hi!
If no one objects, I plan on merging
https://github.com/willfitch/php-src/commit/06170d344f6b3148d505afd8ae952d3439de9005
to 5.3 and up later tonight. This bug has been out there for a long
time.
I see my comments on the patch were ignored. Could you explain what
happens if the password is, for example, 'foo? From the patch, it looks
like it would be placed there as-is. Is this the correct behavior?
Also, it this a security issue? Because 5.3 is security fixes only.
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227
Hi Stas,
I see no comments from you. The only response I've seen from you was
your asking me if the patch was ready (in comments on the bug). Can you
please elaborate?
This isn't technically a security issue, but the bug was reported on
5.3, and affects 5.3. If this is an issue, I can revert the 5.3 change,
but this was a miscommunication between Iliaa and myself. We both
originally had patches, and they ended up getting combined.
Hi!
If no one objects, I plan on merging
https://github.com/willfitch/php-src/commit/06170d344f6b3148d505afd8ae952d3439de9005
to 5.3 and up later tonight. This bug has been out there for a long
time.I see my comments on the patch were ignored. Could you explain what
happens if the password is, for example, 'foo? From the patch, it looks
like it would be placed there as-is. Is this the correct behavior?Also, it this a security issue? Because 5.3 is security fixes only.
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227
Hi!
I see no comments from you. The only response I've seen from you was
your asking me if the patch was ready (in comments on the bug). Can you
please elaborate?
The comment is right in the patch:
https://github.com/willfitch/php-src/commit/06170d344f6b3148d505afd8ae952d3439de9005
but I have already repeated it here - from the code it follows that if
the password starts with ' or ends with ', it will not be encoded. Is
this correct behavior?
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227
Hi!
Hi!
I see no comments from you. The only response I've seen from you was
your asking me if the patch was ready (in comments on the bug). Can you
please elaborate?The comment is right in the patch:
https://github.com/willfitch/php-src/commit/06170d344f6b3148d505afd8ae952d3439de9005
but I have already repeated it here - from the code it follows that if
the password starts with ' or ends with ', it will not be encoded. Is
this correct behavior?
My apologies for not seeing the comment in Github. I didn't get notified
by email, so I didn't look. Starting with a quote would technically
work, but ending would not (assuming you're throwing the password in
from the DSN and not parameter). However, I'm not sure this edge is
worth the CPU cycles for checking. Then again, I didn't expect to have
a bug where single quotes are part of the password, so there's always a
surprise.
If you feel it's worth the overhead, I'll add in the additional logic.
--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227
Will Fitch wrote:
Then again, I didn't expect to have
a bug where single quotes are part of the password, so there's always a
surprise.
Leaving holes that can possibly be used by hackers is the problem here. IF
someone finds an edge case that does not get handled their next step is to see
if it can be exploited? Code review is not a matter of 'surprise' but rather
'what have I missed that could be a problem'?
--
Lester Caine - G8HFL
Contact - http://lsces.co.uk/wiki/?page=contact
L.S.Caine Electronic Services - http://lsces.co.uk
EnquirySolve - http://enquirysolve.com/
Model Engineers Digital Workshop - http://medw.co.uk
Rainbow Digital Media - http://rainbowdigitalmedia.co.uk
Will Fitch wrote:
Then again, I didn't expect to have
a bug where single quotes are part of the password, so there's always a
surprise.Leaving holes that can possibly be used by hackers is the problem here.
IF
someone finds an edge case that does not get handled their next step is
to see
if it can be exploited? Code review is not a matter of 'surprise' but
rather
'what have I missed that could be a problem'?
I agree. However, this is more of a situation of not accounting for all
situations as opposed to introducing a security flaw. As I told Stas,
I'm going to update to account for beginning/ending quotes.
--
Lester Caine - G8HFLContact - http://lsces.co.uk/wiki/?page=contact
L.S.Caine Electronic Services - http://lsces.co.uk
EnquirySolve - http://enquirysolve.com/
Model Engineers Digital Workshop - http://medw.co.uk
Rainbow Digital Media - http://rainbowdigitalmedia.co.uk
Will Fitch wrote:
Will Fitch wrote:
Then again, I didn't expect to have
a bug where single quotes are part of the password, so there's always a
surprise.Leaving holes that can possibly be used by hackers is the problem here.
IF
someone finds an edge case that does not get handled their next step is
to see
if it can be exploited? Code review is not a matter of 'surprise' but
rather
'what have I missed that could be a problem'?
I agree. However, this is more of a situation of not accounting for all
situations as opposed to introducing a security flaw. As I told Stas,
I'm going to update to account for beginning/ending quotes.
Many of the edge cases that get missed are quite benign but some of them can be
a surprise. It is perhaps a little surprising how some holes can be exploited,
even when we thought they were safe :(
--
Lester Caine - G8HFL
Contact - http://lsces.co.uk/wiki/?page=contact
L.S.Caine Electronic Services - http://lsces.co.uk
EnquirySolve - http://enquirysolve.com/
Model Engineers Digital Workshop - http://medw.co.uk
Rainbow Digital Media - http://rainbowdigitalmedia.co.uk
Will Fitch wrote:
Will Fitch wrote:
Then again, I didn't expect to have
a bug where single quotes are part of the password, so there's always a
surprise.Leaving holes that can possibly be used by hackers is the problem here.
IF
someone finds an edge case that does not get handled their next step is
to see
if it can be exploited? Code review is not a matter of 'surprise' but
rather
'what have I missed that could be a problem'?
I agree. However, this is more of a situation of not accounting for all
situations as opposed to introducing a security flaw. As I told Stas,
I'm going to update to account for beginning/ending quotes.Many of the edge cases that get missed are quite benign but some of them
can be
a surprise. It is perhaps a little surprising how some holes can be
exploited,
even when we thought they were safe :(
Well said. :)
--
Lester Caine - G8HFLContact - http://lsces.co.uk/wiki/?page=contact
L.S.Caine Electronic Services - http://lsces.co.uk
EnquirySolve - http://enquirysolve.com/
Model Engineers Digital Workshop - http://medw.co.uk
Rainbow Digital Media - http://rainbowdigitalmedia.co.uk
Hi Lester,
Will Fitch wrote:
Will Fitch wrote:
Then again, I didn't expect to have
a bug where single quotes are part of the password, so there's
always a
surprise.Leaving holes that can possibly be used by hackers is the problem
here.
IF
someone finds an edge case that does not get handled their next step
is
to see
if it can be exploited? Code review is not a matter of 'surprise' but
rather
'what have I missed that could be a problem'?
I agree. However, this is more of a situation of not accounting for
all
situations as opposed to introducing a security flaw. As I told Stas,
I'm going to update to account for beginning/ending quotes.Many of the edge cases that get missed are quite benign but some of them
can be
a surprise. It is perhaps a little surprising how some holes can be
exploited,
even when we thought they were safe :(Well said. :)
Good point. Older PostgreSQL uses \ as escape char.
There is standard conforming string handling and it is the default
currently.
However, it's a configurable option. It's safe as long as E'str' is used.
Reference: standard_conforming_strings
http://www.postgresql.org/docs/9.1/static/runtime-config-compatible.html
Is this issue considered?
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi all,
If no one objects, I plan on merging
https://github.com/willfitch/php-src/commit/06170d344f6b3148d505afd8ae952d3439de9005
to 5.3 and up later tonight. This bug has been out there for a long
time.
5.3 is for security fixes, please merge in 5.4+ only.
Cheers,
Pierre
Hi all,
If no one objects, I plan on merging
[2]https://github.com/willfitch/php-src/commit/06170d344f6b3148d505afd8
ae952d3439de9005
to 5.3 and up later tonight. This bug has been out there for a long
time.
5.3 is for security fixes, please merge in 5.4+ only.
Cheers,
Pierre
Thanks, Pierre. I've reverted from 5.3.
References