Re: Forbid use of LF and CR characters in database and role names

From: Noah Misch <noah(at)leadboat(dot)com>
To: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Forbid use of LF and CR characters in database and role names
Date: 2016-09-12 01:01:53
Message-ID: 20160912010133.GA13829@tornado.leadboat.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Sep 08, 2016 at 12:12:49PM -0400, Peter Eisentraut wrote:
> On 9/6/16 1:42 PM, Robert Haas wrote:
> > On Tue, Sep 6, 2016 at 9:43 PM, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> wrote:
> > > Everything that is using appendShellString() is now going to reject LF
> > > and CR characters, but there is no systematic way by which this is
> > > managed, enforced, or documented.

I agree, and I think that's roughly what it deserves. Longstanding use of
MAXPGPATH is worse; we truncate silently and proceed to whatever malfunction
that entails. We do not see complaints about it. To make MAXPGPATH or LF/CR
better-managed would nominally improve PostgreSQL, but the value is low.

I discourage documenting LF/CR restrictions. For the epsilon of readers who
would benefit from this knowledge, the error message suffices. For everyone
else, it would just dilute the text. (One could argue against other parts of
our documentation on this basis, but I'm not calling for such a study. I'm
just saying that today's lack of documentation on this topic is optimal.)

> > > I think the way forward here, if any, is to work on removing these
> > > restrictions, not to keep sprinkling them around.
> >
> > If we were talking about pathnames containing spaces, I would agree,
> > but I've never heard of a legitimate pathname containing CR or LF. I
> > can't see us losing much by refusing to allow such pathnames, except
> > for security holes.

(In other words, +1 to that.)

> The flip side of that is that if we're doing a half-way job of only
> prohibiting these characters in 67% of cases, then a new generation of
> tools will be written on top of that with the assumption that these
> characters cannot appear.

There's some risk there, but it doesn't feel too likely to me. If the current
tools generation had contemplated those characters, we'd have learned of
CVE-2016-5424 earlier. Even so, ...

> The current setup is more robust: We are prohibiting these characters
> in specific locations where we know we can't handle them. But we don't
> give any guarantees about anything else.

... I'd be fine with that conclusion.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2016-09-12 01:07:33 Re: [REVIEW] Tab Completion for CREATE DATABASE ... TEMPLATE ...
Previous Message Michael Paquier 2016-09-12 01:00:50 Re: Re: [COMMITTERS] pgsql: Use LEFT JOINs in some system views in case referenced row doesn