Thread: Re: [HACKERS] Possible make_oidjoins_check Security Issue
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > I believe the proper way to handle this is a new directory under /tmp. > > It's definitely not worth the trouble. I looked at what configure does > to make /tmp subdirectories portably, and it is spectacularly ugly > (not to mention long). If make_oidjoins_check were a user-facing tool > that would be one thing, but it isn't ... >From a public relations perspective and a code reuse perspective I think we should create temporary tables securely. The attached applied patch fixes contrib/findoidjoins/make_oidjoins_check. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073 Index: contrib/findoidjoins/make_oidjoins_check =================================================================== RCS file: /cvsroot/pgsql/contrib/findoidjoins/make_oidjoins_check,v retrieving revision 1.5 diff -c -c -r1.5 make_oidjoins_check *** contrib/findoidjoins/make_oidjoins_check 20 Oct 2004 16:42:46 -0000 1.5 --- contrib/findoidjoins/make_oidjoins_check 3 Nov 2004 22:42:06 -0000 *************** *** 10,21 **** # Caution: you may need to use GNU awk. AWK=${AWK:-awk} ! INPUTFILE="tmp$$a" ! DUPSFILE="tmp$$b" ! NONDUPSFILE="tmp$$c" ! rm -f $INPUTFILE $DUPSFILE $NONDUPSFILE ! trap "rm -f $INPUTFILE $DUPSFILE $NONDUPSFILE" 0 1 2 3 15 # Read input cat "$@" >$INPUTFILE --- 10,32 ---- # Caution: you may need to use GNU awk. AWK=${AWK:-awk} ! TMP="/tmp/$$" ! trap "rm -rf $TMP" 0 1 2 3 15 ! # Create a temporary directory with the proper permissions so no one can ! # intercept our temporary files and cause a security breach. ! OMASK="`umask`" ! umask 077 ! if ! mkdir $TMP ! then echo "Can't create temporary directory $TMP." 1>&2 ! exit 1 ! fi ! umask "$OMASK" ! unset OMASK ! ! INPUTFILE="$TMP/a" ! DUPSFILE="$TMP/b" ! NONDUPSFILE="$TMP/c" # Read input cat "$@" >$INPUTFILE
Bruce Momjian wrote:
> Tom Lane wrote:
> > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > > I believe the proper way to handle this is a new directory under /tmp.
> >
> > It's definitely not worth the trouble. I looked at what configure does
> > to make /tmp subdirectories portably, and it is spectacularly ugly
> > (not to mention long). If make_oidjoins_check were a user-facing tool
> > that would be one thing, but it isn't ...
>
> >From a public relations perspective and a code reuse perspective I think
> we should create temporary tables securely. The attached applied patch
^^^^^^
files
> fixes contrib/findoidjoins/make_oidjoins_check.
Sorry, meant temporary files.
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.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@candle.pha.pa.us> writes:
> From a public relations perspective and a code reuse perspective I think
> we should create temporary tables securely. The attached applied patch
> fixes contrib/findoidjoins/make_oidjoins_check.
... and creates issues of its own, such as attempting an rm -rf on
something that it shouldn't. At the very least don't install the trap
until after creating the directory successfully.
I really think this is a waste of time though. The current code creates
the temp files in the current directory, and if the bad guy has write
access on that directory you are already screwed (for instance, what's
to stop him from altering the script file itself to do anything at all
when you run it?). I do not think that putting stuff back into /tmp is
an improvement; that just adds risks where none exist now.
regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > From a public relations perspective and a code reuse perspective I think > > we should create temporary tables securely. The attached applied patch > > fixes contrib/findoidjoins/make_oidjoins_check. > > ... and creates issues of its own, such as attempting an rm -rf on > something that it shouldn't. At the very least don't install the trap > until after creating the directory successfully. OK, moved. > I really think this is a waste of time though. The current code creates > the temp files in the current directory, and if the bad guy has write > access on that directory you are already screwed (for instance, what's > to stop him from altering the script file itself to do anything at all > when you run it?). I do not think that putting stuff back into /tmp is > an improvement; that just adds risks where none exist now. My method is secure, and I think we do have to handle this in a way that addresses the security concerns. It is easy to say no one would run this under normal use but that isn't really a safe answer for the security folks, I think. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
On Thu, 2004-11-04 at 10:07, Bruce Momjian wrote: > My method is secure, and I think we do have to handle this in a way that > addresses the security concerns. I think Tom's fix adequately addresses the security concerns. Exactly what is wrong with writing to the current working directory? > It is easy to say no one would run > this under normal use but that isn't really a safe answer for the > security folks, I think. This is a non-sequitor -- I don't think Tom or anyone else has argued this. -Neil
Neil Conway wrote: > On Thu, 2004-11-04 at 10:07, Bruce Momjian wrote: > > My method is secure, and I think we do have to handle this in a way that > > addresses the security concerns. > > I think Tom's fix adequately addresses the security concerns. Exactly > what is wrong with writing to the current working directory? Because it could be run from a directory where others have write permission. > > It is easy to say no one would run > > this under normal use but that isn't really a safe answer for the > > security folks, I think. > > This is a non-sequitor -- I don't think Tom or anyone else has argued > this. I remember hearing that from someone. I thought it was Tom. Bottom line is that the only secure way I have ever heard of for creating temp files is to create a 077 directory in /tmp and write in there. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.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@candle.pha.pa.us> writes:
>> I think Tom's fix adequately addresses the security concerns. Exactly
>> what is wrong with writing to the current working directory?
> Because it could be run from a directory where others have write
> permission.
In which case, they could also change the findoidjoins script itself.
I think your fix is *less* secure than what you replaced.
However, I've already wasted more than enough time on this issue...
I'm done arguing about it.
regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > >> I think Tom's fix adequately addresses the security concerns. Exactly > >> what is wrong with writing to the current working directory? > > > Because it could be run from a directory where others have write > > permission. > > In which case, they could also change the findoidjoins script itself. > I think your fix is *less* secure than what you replaced. > > However, I've already wasted more than enough time on this issue... > I'm done arguing about it. As far as I know, my method is the only secure method. If I am wrong I would like to know. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
On Wed, 3 Nov 2004, Bruce Momjian wrote:
> Tom Lane wrote:
> > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > >> I think Tom's fix adequately addresses the security concerns. Exactly
> > >> what is wrong with writing to the current working directory?
> >
> > > Because it could be run from a directory where others have write
> > > permission.
> >
> > In which case, they could also change the findoidjoins script itself.
> > I think your fix is *less* secure than what you replaced.
> >
> > However, I've already wasted more than enough time on this issue...
> > I'm done arguing about it.
>
> As far as I know, my method is the only secure method. If I am wrong I
> would like to know.
I think the problem can really be solved by just removing it from the
distribution. However, one thing I noticed with Bruce's script is that it
does not respect $TMPDIR -- which security conscious admins may be
setting. Solution would be to set TMP=${TMPDIR:-/tmp} before defining the
path to the temporary sub directory.
Thanks,
Gavin
Gavin Sherry <swm@linuxworld.com.au> writes:
> I think the problem can really be solved by just removing it from the
> distribution.
Just FYI, I've already done that in Red Hat's RPMs (not sure if Devrim
followed suit). I can't think of a good reason for "make install" to
install that script, either.
regards, tom lane
Gavin Sherry wrote:
> On Wed, 3 Nov 2004, Bruce Momjian wrote:
>
> > Tom Lane wrote:
> > > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > > >> I think Tom's fix adequately addresses the security concerns. Exactly
> > > >> what is wrong with writing to the current working directory?
> > >
> > > > Because it could be run from a directory where others have write
> > > > permission.
> > >
> > > In which case, they could also change the findoidjoins script itself.
> > > I think your fix is *less* secure than what you replaced.
> > >
> > > However, I've already wasted more than enough time on this issue...
> > > I'm done arguing about it.
> >
> > As far as I know, my method is the only secure method. If I am wrong I
> > would like to know.
>
> I think the problem can really be solved by just removing it from the
> distribution. However, one thing I noticed with Bruce's script is that it
> does not respect $TMPDIR -- which security conscious admins may be
> setting. Solution would be to set TMP=${TMPDIR:-/tmp} before defining the
> path to the temporary sub directory.
OK, TMPDIR honored. Thanks.
I am fine with removing it but if we don't I would like to have it
secure, mostly from a public relations perspective.
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
On Thu, 2004-11-04 at 13:05, Bruce Momjian wrote: > I am fine with removing it but if we don't I would like to have it > secure, mostly from a public relations perspective. A change which introduced two regressions and fails to materially improve the security of the script is a curious definition of "secure" if you ask me... Attached is a patch that removes the make_oidjoins_check script from "make install". Barring any objections, I'll apply it to HEAD later today. -Neil
Attachment
Neil Conway <neilc@samurai.com> writes:
> Attached is a patch that removes the make_oidjoins_check script from
> "make install". Barring any objections, I'll apply it to HEAD later
> today.
If we are going in that direction, all the files installed by this
subdirectory should be suppressed (ie, findoidjoins and
README.findoidjoins too).
regards, tom lane
Tom Lane wrote: > If we are going in that direction, all the files installed by this > subdirectory should be suppressed (ie, findoidjoins and > README.findoidjoins too). Why not move it to src/tools, so no one gets the impression that it is user code? -- Peter Eisentraut http://developer.postgresql.org/~petere/
Peter Eisentraut <peter_e@gmx.net> writes:
> Why not move it to src/tools, so no one gets the impression that it is
> user code?
I thought about that earlier, but concluded it wasn't worth the loss of
CVS history.
regards, tom lane
On Thu, Nov 04, 2004 at 09:47:46AM -0500, Tom Lane wrote: > Peter Eisentraut <peter_e@gmx.net> writes: > > Why not move it to src/tools, so no one gets the impression that it is > > user code? > > I thought about that earlier, but concluded it wasn't worth the loss of > CVS history. I have counted three times you have said that in the recent past. IMHO this really screams of changing the SCM tool. Can this be discussed for 8.1? -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) "La felicidad no es mañana. La felicidad es ahora"
Alvaro Herrera <alvherre@dcc.uchile.cl> writes:
> Can this be discussed for 8.1?
It's been discussed, and rejected, several times already. There aren't
any alternatives that are enough better than CVS to be worth the
changeover effort.
regards, tom lane
On Thu, 4 Nov 2004, Alvaro Herrera wrote: > On Thu, Nov 04, 2004 at 09:47:46AM -0500, Tom Lane wrote: >> Peter Eisentraut <peter_e@gmx.net> writes: >>> Why not move it to src/tools, so no one gets the impression that it is >>> user code? >> >> I thought about that earlier, but concluded it wasn't worth the loss of >> CVS history. why would we lose CVS history? I can physically move the files in /cvsroot to accomplish this ... just tell me what needs to move, and to where ... ---- Marc G. Fournier Hub.Org Networking Services (http://www.hub.org) Email: scrappy@hub.org Yahoo!: yscrappy ICQ: 7615664
"Marc G. Fournier" <scrappy@postgresql.org> writes:
> why would we lose CVS history? I can physically move the files in
> /cvsroot to accomplish this ... just tell me what needs to move, and to
> where ...
If you physically move the files, that would retroactively change their
placement in back versions, no? ie, it would appear that all previous
releases had had 'em under src/tools as well.
AFAICS the only nondestructive way to do this is to cvs delete and cvs
add, with a commit comment saying where the files were moved from. Then
when you are looking at them in CVS, you'd have to navigate over to the
previous location (by hand, probably; the commit comment isn't going to
automate this for you) and look in the Attic to read the prior CVS history.
It's not impossible, certainly, but it discourages moving files for less
than the very best of reasons.
(I'm rather interested to know whether any other SCMs have a better
solution to this problem, and if so what it is. It's not obvious how
to do better.)
regards, tom lane
On Thu, 4 Nov 2004, Tom Lane wrote: > "Marc G. Fournier" <scrappy@postgresql.org> writes: >> why would we lose CVS history? I can physically move the files in >> /cvsroot to accomplish this ... just tell me what needs to move, and to >> where ... > > If you physically move the files, that would retroactively change their > placement in back versions, no? ie, it would appear that all previous > releases had had 'em under src/tools as well. Erk, yes, good point ... ---- Marc G. Fournier Hub.Org Networking Services (http://www.hub.org) Email: scrappy@hub.org Yahoo!: yscrappy ICQ: 7615664
Tom Lane wrote: > AFAICS the only nondestructive way to do this is to cvs delete and cvs > add, with a commit comment saying where the files were moved from. Then > when you are looking at them in CVS, you'd have to navigate over to the > previous location (by hand, probably; the commit comment isn't going to > automate this for you) and look in the Attic to read the prior CVS history. > It's not impossible, certainly, but it discourages moving files for less > than the very best of reasons. You can also do a repository-side copy of the ,v file to the new location, remove old tags & branches from that new copy, and 'cvs delete' the old copy. That preserves history but the file should still show up in the old location (and not also in the new location) when older versions are checked out. In theory. It's all very hairy.. > (I'm rather interested to know whether any other SCMs have a better > solution to this problem, and if so what it is. It's not obvious how > to do better.) Subversion deals with this reasonably well. The main difference to CVS is that it does not try to track multiple lines of development in a particular file; instead, you make (internally cheap) copies *within* the repository tree when you branch or tag. Once you have that, it's much easier to track file copies and deletions, as each path in the repository effectively has a linear history. A rename is just a copy and delete. See http://svnbook.red-bean.com/svnbook-1.0/ch04s02.html for some more detail. -O
Hi, On Thursday 04 November 2004 20:41, Tom Lane wrote: > "Marc G. Fournier" <scrappy@postgresql.org> writes: > > why would we lose CVS history? I can physically move the files in > > /cvsroot to accomplish this ... just tell me what needs to move, and to > > where ... > > If you physically move the files, that would retroactively change their > placement in back versions, no? ie, it would appear that all previous > releases had had 'em under src/tools as well. > > AFAICS the only nondestructive way to do this is to cvs delete and cvs > add, with a commit comment saying where the files were moved from. Then > when you are looking at them in CVS, you'd have to navigate over to the > previous location (by hand, probably; the commit comment isn't going to > automate this for you) and look in the Attic to read the prior CVS history. > It's not impossible, certainly, but it discourages moving files for less > than the very best of reasons. > > (I'm rather interested to know whether any other SCMs have a better > solution to this problem, and if so what it is. It's not obvious how > to do better.) > > regards, tom lane > > ---------------------------(end of broadcast)--------------------------- > TIP 8: explain analyze is your friend <Advocacy> Yes, some do. At least SVN (Subversion) can handle moves very well, and especially it doesn't loose history on moves/renames. SVN holds it's repo entries in a database like 'filesystem', which can be backed by BDB4 or flat files (as of 1.1). SVN has proven to be stable in many OSS projects, and vastly superior over CVS especially in handling multi-GB projects containing binary files in our company. I refrain from listing all the advantages, if interested, have a look for yourself at http://subversion.tigris.org </Advocacy> Having one file in the repo per working copy file like with CVS is an obvious, but also obviously limited approach. Greetings, Jörg -- Leading SW developer - S.E.A GmbH Mail: joerg.hessdoerfer@sea-gmbh.com WWW: http://www.sea-gmbh.com
Tom, > (I'm rather interested to know whether any other SCMs have a better > solution to this problem, and if so what it is. It's not obvious how > to do better.) > I've been working with a few SCM's and IMHO only one of them really handles this really well. That's ClearCase. I'm well aware that ClearCase is not an option but I though it could still be interesting to know how this can be managed when done right. In ClearCase everything (both files and directories) are "elements". A directory is a version of an element and it contains versions of other elements. It's not very different from Unix and I-nodes although everything is of course versioned. Subversion claims they handle moves pretty well. When I checked it out, it turns out that a move is a copy (versions and all) followed by a delete, thus maintaining version history at both locations. I'd recommend anyone who think CVS is insufficient due to file moves to investigate subversion. Regards, Thomas Hallgren
On Fri, 5 Nov 2004 07:02 am, Marc G. Fournier wrote: > On Thu, 4 Nov 2004, Tom Lane wrote: > > > "Marc G. Fournier" <scrappy@postgresql.org> writes: > >> why would we lose CVS history? I can physically move the files in > >> /cvsroot to accomplish this ... just tell me what needs to move, and to > >> where ... > > > > If you physically move the files, that would retroactively change their > > placement in back versions, no? ie, it would appear that all previous > > releases had had 'em under src/tools as well. > > Erk, yes, good point ... You could always, physically copy the file to the new location. Giving you all the history in the new location and run CVS delete on the only location. I can't see how this is too different from the cvs remove/cvs add. However you get to keep the history as well as keeping the old version. The second problem still exists where it's in 2 locations in previous releases. unless you cvs remove the new copy from those branches as well. As always CVS is a bit messy with these things, but just throwing ideas on the pile that might work. Regards Russell Smith
Tom Lane wrote: > Alvaro Herrera <alvherre@dcc.uchile.cl> writes: > >>Can this be discussed for 8.1? > > > It's been discussed, and rejected, several times already. There aren't > any alternatives that are enough better than CVS to be worth the > changeover effort. The effort is not so big: http://cvs2svn.tigris.org Do not rename or move around a file because your SCM limits, is insane. Regards Gaetano Mendola
In an attempt to throw the authorities off his trail, tgl@sss.pgh.pa.us (Tom Lane) transmitted:
> Alvaro Herrera <alvherre@dcc.uchile.cl> writes:
>> Can this be discussed for 8.1?
>
> It's been discussed, and rejected, several times already. There
> aren't any alternatives that are enough better than CVS to be worth
> the changeover effort.
Subversion may be getting close to the point where it may be worth
thinking of, and there is a pretty full-featured conversion scheme,
cvs2svn, allowing considerable choice as to what aspects of the CVS
branches will be included.
The one traditional _enormous_ problem with it was that while
much-lauded, it suffered interoperability issues. People running
different versions of {Debian|RHAT|FreeBSD|...} could have versions
that couldn't talk to one another. That appears to have been
alleviated:
"Now that subversion has reached 1.0.0 our compatibility guarantees
require forward and backward compatible repository formats for all
patch releases and backward compatible for minor releases. So
until 2.0.0 comes out there will be no change that should require a
dump for upgrading to newer versions."
I'll buy the argument that it'll take some work for people familiar
with CVS to get familiar with SVN. Of course...
"Generally, Subversion's interface to a particular feature is
similar to CVS's, except where there's a compelling reason to do
otherwise."
I have been watching Subversion develop for quite some time, and have
always felt it the right idea to put usage off because it did not
appear mature enough. I have always thought "in another year, it may
be ready." As far as maturity is concerned, it looks like it's there
now. The formerly compelling reasons for instant rejection are no
longer there.
If it's plausible to run a SVN archive, in parallel, that can accept
patches coming out of the present CVS, it must surely be time for some
intrepid fan of Subversion to put up an an archive and start showing
off how much better it is. Proving it's viable by demonstration is a
pretty ideal methodology, no?
By the way, one of the longer term goals is for SVN to support a SQL
repository backend; there's probably merit to some "common dogfood
usage" ;-).
--
(reverse (concatenate 'string "gro.gultn" "@" "enworbbc"))
http://linuxfinances.info/info/unix.html
"Are we worried about Linux? ... Sure we are worried."
-- Steve Ballmer, VP of MICROS~1 at Seybold publishing conference
The intuitive understanding of a file is certainly something like "a file called 'baz.c' residing at 'foo/bar/', which contains the BAZ subsystem". Now, when renaming/moving a file such an intuitive understanding is partially lost. UI-wise that's a problem which I haven't ever seen solved well. However, other SCM systems such as Subversion and Continuus (and our to-be-released system Maint, and certainly others) treat files as unique entities unrelated to their path, and thus don't have problems with moves. With regards to modes of working this, it boils down to two methods. One is treating directories as first class entities (opposed to CVS which treats dirs as semi-relevant appendices to real files), versioned to contain a list of children, or simpler yet, to store the parent directory as an meaningful attribute of an object. Both methods have their pros and cons, the latter is somehow simpler to intuitively grasp for people. This doesn't really answer the question of what tool Postgres might change to, but it seems that Subversion is a good tool one should consider. And by golly, CVS is bad. Just consider the cons – having to forbid renames in all but the most necessary cases – it just invites cruft into any project. d. -- David Helgason, Business Development et al., Over the Edge I/S (http://otee.dk) Direct line +45 2620 0663 Main line +45 3264 5049 On 4. nov 2004, at 20:41, Tom Lane wrote: > "Marc G. Fournier" <scrappy@postgresql.org> writes: >> why would we lose CVS history? I can physically move the files in >> /cvsroot to accomplish this ... just tell me what needs to move, and >> to >> where ... > > If you physically move the files, that would retroactively change their > placement in back versions, no? ie, it would appear that all previous > releases had had 'em under src/tools as well. > > AFAICS the only nondestructive way to do this is to cvs delete and cvs > add, with a commit comment saying where the files were moved from. > Then > when you are looking at them in CVS, you'd have to navigate over to the > previous location (by hand, probably; the commit comment isn't going to > automate this for you) and look in the Attic to read the prior CVS > history. > It's not impossible, certainly, but it discourages moving files for > less > than the very best of reasons. > > (I'm rather interested to know whether any other SCMs have a better > solution to this problem, and if so what it is. It's not obvious how > to do better.) > > regards, tom lane > > ---------------------------(end of > broadcast)--------------------------- > TIP 8: explain analyze is your friend >
Joerg Hessdoerfer wrote: > <Advocacy> > Yes, some do. At least SVN (Subversion) can handle moves very well, and > especially it doesn't loose history on moves/renames. > SVN holds it's repo entries in a database like 'filesystem', which can be > backed by BDB4 or flat files (as of 1.1). > SVN has proven to be stable in many OSS projects, and vastly superior over CVS > especially in handling multi-GB projects containing binary files in our > company. > > I refrain from listing all the advantages, if interested, have a look for > yourself at http://subversion.tigris.org > > </Advocacy> <MoreAdvocacy> Another compelling reason to use SVN is that one of their long term goals is to use an SQL backend. PostreSQL must be the absolute best choice for that, right? So knowledge of SVN and some future collaboration could perhaps be beneficial for both parties. SVN is also targeted as a CVS replacement and a CVS user will feel very much at home. </MoreAdvocacy> Regards, Thomas Hallgren
Thomas Hallgren wrote: > Another compelling reason to use SVN is that one of their long term > goals is to use an SQL backend. That is about as far from a "compelling reason" to use a particular version control system as I can imagine. -Neil
> This doesn't really answer the question of what tool Postgres might > change to, but it seems that Subversion is a good tool one should > consider. And by golly, CVS is bad. Just consider the cons – having > to forbid renames in all but the most necessary cases – it just > invites cruft into any project. Interesting reading: http://better-scm.berlios.de/comparison/comparison.html http://zooko.com/revision_control_quick_ref.html Cheers, Steve