Hi!
On Mar 13, Dmitry V. Levin wrote:
> On Thu, Mar 13, 2003 at 06:25:59PM +0100, Sergei Golubchik wrote:
> > > > > > So I am not sure, if we would gain anything here by using
> fstat()
> > > > >
> > > > > You'll get a bit more secure code. :)
> > > >
> > > > I cannot see how fstat() can make the code even slightly secure.
> > > > What is the scenario where changing stat() to fstat() can help ?
> > >
> > > It's quite common case:
> > > When you call stat before open, there is a race; attacker with rights of
> > > mysql user may subvert the file right after the check, and make root to
> > > follow his evil instructions.
> >
> > Nope - it's just general words, they do not apply to this case.
> > It attacker has rights of mysql nothing prevents him from altering file
> > content and setting proper permissions before any check.
>
> Unless corresponding files and directories are owned by root.
>
> If attacker owns the directory, he can just subvert the file.
> If attacker owns the file, he can subvert its content.
sure, sure.
But still, those issues have nothing to do with race condition.
This particular race condition allows only one thing.
To force MySQL to read world-writeable my.cnf by changing file
permissions between stat() and open() calls.
Of anybody can do that he can also
1. Change file permission to world-writeable and back *before* mysqld is
ever run.
2. Change file permissions to world-writeable *after* open() and fstat()
calls - and then add something to the end of file, so mysqld will read it.
3. Alter file contents without changing its permissions
and so on.
> Using fstat together with st_uid check closes these issues, too.
As I said, we cannot add st_uid check in 3.23 or 4.0.
and, btw, #2 from above shows that using fstat() doesn't close race
condition :)
> After all, let's try to avoid potentially raceable constructions.
This particular construction is not exploitable.
Let's try to apply rules wherever they matter, and not where
pattern-matching tool identifies a "potential vulnerability".
Not every strcpy->strncpy change is meaningful.
Regards,
Sergei
--
MySQL Development Team
__ ___ ___ ____ __
/ |/ /_ __/ __/ __ \/ / Sergei Golubchik <serg@stripped>
/ /|_/ / // /\ \/ /_/ / /__ MySQL AB, http://www.mysql.com/
/_/ /_/\_, /___/\___\_\___/ Osnabrueck, Germany
<___/
