Re: proposal: psql \setfileref - Mailing list pgsql-hackers

From Gilles Darold
Subject Re: proposal: psql \setfileref
Date
Msg-id 098d7646-9ce0-db36-d52d-58b7b879a299@dalibo.com
Whole thread Raw
In response to Re: proposal: psql \setfileref  (Pavel Stehule <pavel.stehule@gmail.com>)
Responses Re: proposal: psql \setfileref  ("Daniel Verite" <daniel@manitou-mail.org>)
List pgsql-hackers
Le 09/10/2016 à 11:48, Pavel Stehule a écrit :
hi

2016-10-04 9:18 GMT+02:00 Gilles Darold <gilles.darold@dalibo.com>:
Le 03/10/2016 à 23:23, Gilles Darold a écrit :
> Le 03/10/2016 à 23:03, Robert Haas a écrit :
>> On Mon, Oct 3, 2016 at 3:54 PM, Gilles Darold <gilles@darold.net> wrote:
>>> 4) An other problem is that like this this patch will allow anyone to upload into a
>>> column the content of any system file that can be read by postgres system user
>>> and then allow non system user to read its content.
>> I thought this was a client-side feature, so that it would let a
>> client upload any file that the client can read, but not things that
>> can only be read by the postgres system user.
>>
> Yes that's right, sorry for the noise, forget this fourth report.
>

After some more though there is still a security issue here. For a
PostgreSQL user who also have login acces to the server, it is possible
to read any file that the postgres system user can read, especially a
.pgpass or a recovery.conf containing password.

here is new update - some mentioned issues are fixed + regress tests and docs

Looks very good for me minus the two following points:

1) I think \setfileref must return the same syntax than \set command
postgres=# \setfileref a testfile.txt
postgres=# \setfileref
a = 'testfile.txt'

postgres=# \setfileref
...
a = ^'testfile.txt'

I think it would be better to prefixed the variable value with the ^ too like in the \set report even if we know by using this command that reported variables are file references.


2) You still allow special file to be used, I understand that this is from the user responsibility but I think it could be a wise precaution. 

postgres=# \setfileref b /dev/random
postgres=# insert into test (:b);

Process need to be killed using SIGTERM.

However if this last point is not critical and should be handle by the user, I think this patch is ready to be reviewed by a committer after fixing the first point.

Regards,

-- 
Gilles Darold
Consultant PostgreSQL
http://dalibo.com - http://dalibo.org

pgsql-hackers by date:

Previous
From: Heikki Linnakangas
Date:
Subject: Re: Logical tape pause/resume
Next
From: Heikki Linnakangas
Date:
Subject: Re: Logical tape pause/resume