Fwd: Bug#372115: Last security update of postgresql-contrib breaks database replication with DBMirror.pl
Hi PostgreSQL gurus,
we recently received this bug report after we upgraded Debian's stable
release to the equivalent of 7.4.13 (with the fixes for quote
escaping).
Does anyone know DBMirror.pl? The proposed fix seems wrong since it
just reverts the behavior to the old quote escaping style.
Thank you in advance for any idea,
Martin
----- Forwarded message from Olivier Bornet <Olivier.Bornet@puck.ch> -----
Subject: Bug#372115: Last security update of postgresql-contrib breaks
database replication with DBMirror.pl
Reply-To: Olivier Bornet <Olivier.Bornet@puck.ch>, 372115@bugs.debian.org
From: Olivier Bornet <Olivier.Bornet@puck.ch>
To: Debian Bug Tracking System <submit@bugs.debian.org>
Date: Thu, 08 Jun 2006 12:30:55 +0200
X-Spam-Status: No, score=1.3 required=4.0 tests=BAYES_50,DATE_IN_FUTURE_06_12
autolearn=no version=3.0.3
Package: postgresql-contrib
Version: 7.4.7-6sarge2
Severity: critical
Justification: causes serious data loss
Hello,
using version 7.4.7-6sarge2 of postgresql-contrib cause trouble in
database replication using /usr/lib/postgresql/bin/DBMirror.pl
The problem I have found is if there is a ' character (the single quote)
in the data. In this case, the single quote (') is replaced by two
single quotes ('') in the table PendingData. This cause the replication
process to stop with a message "Error in PendingData Sequence Id XXX".
To replicate the non-replicated data, I have run a patched version of
DBMirror.pl. Here is my patch (mainly replacing the two single quotes by
a backslash and one single quote, this mean '' -> \'. Execepted if there
is a equal before, this mean don't replace ='') :
--- /usr/lib/postgresql/bin/DBMirror.pl 2005-05-18 10:33:34.000000000 +0200
+++ ./DBMirror.pl 2006-06-08 11:53:39.000000000 +0200
@@ -827,6 +827,9 @@
$fnumber = 4;
my $dataField = $pendingResult->getvalue($currentTuple,$fnumber);+ # replace all the '' to \' in the texts
+ $dataField =~ s/([^=])\'\'/\1\\\'/g;
+
while(length($dataField)>0) {
# Extract the field name that is surronded by double quotes
$dataField =~ m/(\".*?\")/s;I'm sure this patch is not enough, because this don't take in account if
the data has in it something like "=''". I think the part to patch is
not the DBMirror.pl, but the "recordchange" procedure called by the
trigger on each data change.
Reverting postgresql-contrib to version 7.4.7-6sarge1 correct the
problem only if you have nothing in the Pending table.
Thanks for your attention, and have a nice day.
Oliver
----- End forwarded message -----
--
Martin Pitt http://www.piware.de
Ubuntu Developer http://www.ubuntu.com
Debian Developer http://www.debian.org
In a world without walls and fences, who needs Windows and Gates?
Martin Pitt wrote:
-- Start of PGP signed section.
Hi PostgreSQL gurus,
we recently received this bug report after we upgraded Debian's stable
release to the equivalent of 7.4.13 (with the fixes for quote
escaping).Does anyone know DBMirror.pl? The proposed fix seems wrong since it
just reverts the behavior to the old quote escaping style.
Using '' instead of \' allows dbmirror work now that \' throws an error
on unsafe encodings.
--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Martin Pitt <martin@piware.de> writes:
Does anyone know DBMirror.pl? The proposed fix seems wrong since it
just reverts the behavior to the old quote escaping style.
I don't know it, but the function being complained of seems exactly the
sort of ad-hoc escaping logic that the security update warns you should
get rid of. (I fear we failed to notice it because it was in Perl not C
:-() I think it should be rewritten from the ground up. Does the Pg
Perl module expose PQescapeString by any chance? Relying on that would
be far better than letting this code live.
regards, tom lane
Hi PostgreSQL gurus, hi Olivier,
Martin Pitt [2006-06-16 0:15 +0200]:
Upstream confirmed my reply in the last mail in [1]: the complete
escaping logic in DBMirror.pl is seriously screwew.[1] http://archives.postgresql.org/pgsql-bugs/2006-06/msg00065.php
I finally found some time to debug this, and I think I found a better
patch than the one you proposed. Mine is still hackish and is still a
workaround around a proper quoting solution, but at least it repairs
the parsing without introducing the \' quoting again.
I consider this a band-aid patch to fix the recent security update.
PostgreSQL gurus, would you consider applying this until a better
solution is found for DBMirror.pl?
Olivier, can you please confirm that the patch works for you, too?
Thank you,
Martin
--
Martin Pitt http://www.piware.de
Ubuntu Developer http://www.ubuntu.com
Debian Developer http://www.debian.org
In a world without walls and fences, who needs Windows and Gates?
Attachments:
DBMirror.pl-fixquoteparsing.difftext/plain; charset=us-asciiDownload+1-1
Import Notes
Reply to msg id not found: 20060615221528.GE20421@piware.deReference msg id not found: E1FoHmx-0003mt-00@www.rmhsa.chReference msg id not found: 20060610160358.GC5199@piware.deReference msg id not found: 20060615221528.GE20421@piware.de | Resolved by subject fallback
Martin Pitt <martin@piware.de> writes:
I finally found some time to debug this, and I think I found a better
patch than the one you proposed. Mine is still hackish and is still a
workaround around a proper quoting solution, but at least it repairs
the parsing without introducing the \' quoting again.
Yeah, this is probably all right. My concerns about encoding
vulnerabilities were likely overblown --- it would only be an issue if
the mirror script were running with a non-ASCII-safe client encoding,
which seems pretty unlikely. So this will do as a band aid.
However, in looking through DBMirror.pl to try to understand what was
going on, I immediately found several other bugs --- fails on field
names containing double quotes, mirrorDelete fails to re-quote values,
mirrorUpdate tries to use "field = null" where "field is null" would be
correct, for example. I'm wondering whether this thing is really still
used in practice, and whether we shouldn't be deprecating it in favor of
Slony. As far as I can tell from the CVS logs, dbmirror per se hasn't
been touched since 2004 --- all subsequent edits have been part of
tree-wide changes.
regards, tom lane
Your patch has been added to the PostgreSQL unapplied patches list at:
http://momjian.postgresql.org/cgi-bin/pgpatches
It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.
---------------------------------------------------------------------------
Martin Pitt wrote:
-- Start of PGP signed section.
Hi PostgreSQL gurus, hi Olivier,
Martin Pitt [2006-06-16 0:15 +0200]:
Upstream confirmed my reply in the last mail in [1]: the complete
escaping logic in DBMirror.pl is seriously screwew.[1] http://archives.postgresql.org/pgsql-bugs/2006-06/msg00065.php
I finally found some time to debug this, and I think I found a better
patch than the one you proposed. Mine is still hackish and is still a
workaround around a proper quoting solution, but at least it repairs
the parsing without introducing the \' quoting again.I consider this a band-aid patch to fix the recent security update.
PostgreSQL gurus, would you consider applying this until a better
solution is found for DBMirror.pl?Olivier, can you please confirm that the patch works for you, too?
Thank you,
Martin
--
Martin Pitt http://www.piware.de
Ubuntu Developer http://www.ubuntu.com
Debian Developer http://www.debian.orgIn a world without walls and fences, who needs Windows and Gates?
[ Attachment, skipping... ]
-- End of PGP section, PGP failed!
--
Bruce Momjian bruce@momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Patch applied. Thanks. Backpatched back to 7.3.X.
---------------------------------------------------------------------------
Martin Pitt wrote:
-- Start of PGP signed section.
Hi PostgreSQL gurus, hi Olivier,
Martin Pitt [2006-06-16 0:15 +0200]:
Upstream confirmed my reply in the last mail in [1]: the complete
escaping logic in DBMirror.pl is seriously screwew.[1] http://archives.postgresql.org/pgsql-bugs/2006-06/msg00065.php
I finally found some time to debug this, and I think I found a better
patch than the one you proposed. Mine is still hackish and is still a
workaround around a proper quoting solution, but at least it repairs
the parsing without introducing the \' quoting again.I consider this a band-aid patch to fix the recent security update.
PostgreSQL gurus, would you consider applying this until a better
solution is found for DBMirror.pl?Olivier, can you please confirm that the patch works for you, too?
Thank you,
Martin
--
Martin Pitt http://www.piware.de
Ubuntu Developer http://www.ubuntu.com
Debian Developer http://www.debian.orgIn a world without walls and fences, who needs Windows and Gates?
[ Attachment, skipping... ]
-- End of PGP section, PGP failed!
--
Bruce Momjian bruce@momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +