Thread: Possible make_oidjoins_check Security Issue
On Wed, 2004-10-20 at 06:18, Rod Taylor wrote: > http://secunia.com/advisories/12860/ This seems like a rather inconsequential problem, but it should be fixed. The first two ideas that come to mind: use temporary files in $PWD rather than /tmp, or create a subdirectory in /tmp to use for the temporary files. -Neil
On Wed, Oct 20, 2004 at 12:52:57PM +1000, Neil Conway wrote: > On Wed, 2004-10-20 at 06:18, Rod Taylor wrote: > > http://secunia.com/advisories/12860/ > > This seems like a rather inconsequential problem, but it should be > fixed. The first two ideas that come to mind: use temporary files in > $PWD rather than /tmp, or create a subdirectory in /tmp to use for the > temporary files. Better, use mktemp(1). The thread testing script already does it IIRC. -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) "Un poeta es un mundo encerrado en un hombre" (Victor Hugo)
Neil Conway <neilc@samurai.com> writes: > On Wed, 2004-10-20 at 06:18, Rod Taylor wrote: >> http://secunia.com/advisories/12860/ > This seems like a rather inconsequential problem, Indeed, since ordinary users have no use for make_oidjoins_check. It's surely very implausible that anyone would run it as root; and even if someone did, the attacker cannot control what gets written. > but it should be fixed. The first two ideas that come to mind: use > temporary files in $PWD rather than /tmp, or create a subdirectory in > /tmp to use for the temporary files. I believe that the subdirectory idea is also vulnerable without great care. My inclination so far as the Red Hat packages are concerned is simply to omit the contrib/findoidjoins files from the installed RPMs. The patch originally proposed by trustix involved using mktemp(1), which would be a great fix if mktemp(1) weren't so laughably unportable :-( But in any case it's hard to see why we are expending RPM distro space on this script in the first place. I suspect that no one on the planet except Bruce and myself have ever actually run this script. regards, tom lane
Alvaro Herrera <alvherre@dcc.uchile.cl> writes:
> Better, use mktemp(1). The thread testing script already does it IIRC.
There are only two uses of mktemp(1) in our source tree: configure and
config.guess. Both were gotten from elsewhere, and both jump through
some seriously unreadable hoops in order to achieve allegedly-portable
behavior. mktemp(1) is simply not portable :-( ... the Single Unix Spec
refuses to touch it at all ...
regards, tom lane
On Wed, Oct 20, 2004 at 12:31:11AM -0400, Tom Lane wrote: > Alvaro Herrera <alvherre@dcc.uchile.cl> writes: > > Better, use mktemp(1). The thread testing script already does it IIRC. > > There are only two uses of mktemp(1) in our source tree: configure and > config.guess. Both were gotten from elsewhere, and both jump through > some seriously unreadable hoops in order to achieve allegedly-portable > behavior. Huh, right. I was remembering mkstemp(3), which is used in the thread test (which is not a script after all ...) config.guess usage surely is ugly ... -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) "Porque francamente, si para saber manejarse a uno mismo hubiera que rendir examen... ¿Quién es el machito que tendría carnet?" (Mafalda)
Tom Lane wrote: >I suspect that no one on the planet >except Bruce and myself have ever actually run this script. > > > > Then why don't we just remove it? Problem solved ... cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes:
> Tom Lane wrote:
>> I suspect that no one on the planet
>> except Bruce and myself have ever actually run this script.
> Then why don't we just remove it? Problem solved ...
Because it's a needed maintenance tool. There isn't any particularly
good reason for it to get installed as though it were an interesting
program for users, though, so I think that this is mostly a matter of
poor packaging choices. I am in fact intending to remove the
contrib/findoidjoins files from the set of stuff installed by Red Hat's
RPMs.
I suppose you could make an argument for moving this directory out of
contrib and putting it under src/tools instead, but that seems like more
work (and loss of CVS history) than it's worth.
regards, tom lane
Tom Lane wrote: >Andrew Dunstan <andrew@dunslane.net> writes: > > >>Tom Lane wrote: >> >> >>>I suspect that no one on the planet >>>except Bruce and myself have ever actually run this script. >>> >>> > > > >>Then why don't we just remove it? Problem solved ... >> >> > >Because it's a needed maintenance tool. There isn't any particularly >good reason for it to get installed as though it were an interesting >program for users, though, so I think that this is mostly a matter of >poor packaging choices. I am in fact intending to remove the >contrib/findoidjoins files from the set of stuff installed by Red Hat's >RPMs. > >I suppose you could make an argument for moving this directory out of >contrib and putting it under src/tools instead, but that seems like more >work (and loss of CVS history) than it's worth. > > > > Then maybe there's a case for removing findoidjoins from WANTED_DIRS in contrib/Makefile? I agree this issue is so trifling that it's not worth spending much energy on. On a very slightly related note, I see that ipcclean (which is a shell script) is installed on Windows by "make install". Do we want to fix that or trust the binary packagers to remove it? cheers andrew
Tom Lane wrote: > Neil Conway <neilc@samurai.com> writes: > > On Wed, 2004-10-20 at 06:18, Rod Taylor wrote: > >> http://secunia.com/advisories/12860/ > > > This seems like a rather inconsequential problem, > > Indeed, since ordinary users have no use for make_oidjoins_check. > It's surely very implausible that anyone would run it as root; and > even if someone did, the attacker cannot control what gets written. > > > but it should be fixed. The first two ideas that come to mind: use > > temporary files in $PWD rather than /tmp, or create a subdirectory in > > /tmp to use for the temporary files. > > I believe that the subdirectory idea is also vulnerable without great > care. I believe the proper way to handle this is a new directory under /tmp. I use this in my local scripts: TMP=/tmp/$$OMASK="`umask`"umask 077if ! mkdir "$TMP"then echo "Can't create temporary directory $TMP." 1>&2 exit1fiumask "$OMASK"unset OMASK It basically makes sure it creates a new directory under /tmp with a umask that guarantees no one else can create a file in that directory. All temp files are accessed as $TMP/XXX. -- 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, Pennsylvania19073
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 ...
regards, tom lane