Re: Bug#583435: rpcbind: Insecure handling of state files

Re: Bug#583435: rpcbind: Insecure handling of state files

[Aníbal Monsalve Salazar]

On Tue, Jun 01, 2010 at 02:09:07PM +0200, Guillem Jover wrote:
>Hi!
>
>On Thu, 2010-05-27 at 19:09:08 +0200, Guillem Jover wrote:
>>Package: rpcbind
>>Version: 0.2.0-4
>>Severity: serious
>>Tags: security
>
>>The rpcbind daemon, which runs as root, uses /tmp/portmap.xdr and
>>/tmp/rpcbind.xdr for doing warm starts as what seems to be a way to
>>preserve state between invokations. It parses (through libtirpc) and
>>removes them on start. It creates them before exiting.
>>
>>So first off, *any* user can craft those two files before the daemon
>>has started for the first time, which the daemon will parse. This
>>might be ok, depending on the checks done on parse, I'd still be very
>>wary of letting a user be able to craft such files at will.
>
>It seems to be doing no checks whatsoever. A simple test I performed at
>the time of filing this report, but didn't seem to have any obvious
>consequence, shows this which I noticed later on:
>
>,---
>gaara:~# /etc/init.d/rpcbind start
>Starting rpcbind daemon....
>gaara:~# ps axuOp|egrep '(^USER|[r]pcbind)'
>USER       PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
>root     23424  0.0  0.0  18768   704 ?        Ss   13:53   0:00 /sbin/rpcbind -w
>gaara:~# /etc/init.d/rpcbind stop
>Stopping rpcbind daemon....
>gaara:~# dd if=/dev/urandom of=/tmp/rpcbind.xdr bs=1024 count=1
>1+0 records in
>1+0 records out
>1024 bytes (1,0 kB) copied, 0,000861307 s, 1,2 MB/s
>gaara:~# /etc/init.d/rpcbind start
>Starting rpcbind daemon....
>gaara:~# ps axuOp|egrep '(^USER|[r]pcbind)'
>USER       PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
>root     23440  0.0  0.0 4008972  772 ?        Ss   13:54   0:00 /sbin/rpcbind -w
>`---
>
>The first start is a normal clean invokation, the second one is using
>the crafted file. See how it has allocated almost 4 GiB. Disregard though,
>me running all this as root, a user would be able to craft those files as
>long as they were not already in /tmp.
>
>thanks,
>guillem

I'm sending this bug report to the linux-nfs mailing list.

The original bug report is at http://bugs.debian.org/583435

Thank you.

Re: Bug#583435: rpcbind: Insecure handling of state files

[Chuck Lever]

On 06/ 2/10 07:25 AM, An=EDbal Monsalve Salazar wrote:
> On Tue, Jun 01, 2010 at 02:09:07PM +0200, Guillem Jover wrote:
>> Hi!
>>
>> On Thu, 2010-05-27 at 19:09:08 +0200, Guillem Jover wrote:
>>> Package: rpcbind
>>> Version: 0.2.0-4
>>> Severity: serious
>>> Tags: security
>>
>>> The rpcbind daemon, which runs as root, uses /tmp/portmap.xdr and
>>> /tmp/rpcbind.xdr for doing warm starts as what seems to be a way to
>>> preserve state between invokations. It parses (through libtirpc) an=
d
>>> removes them on start. It creates them before exiting.
>>>
>>> So first off, *any* user can craft those two files before the daemo=
n
>>> has started for the first time, which the daemon will parse. This
>>> might be ok, depending on the checks done on parse, I'd still be ve=
ry
>>> wary of letting a user be able to craft such files at will.
>>
>> It seems to be doing no checks whatsoever. A simple test I performed=
 at
>> the time of filing this report, but didn't seem to have any obvious
>> consequence, shows this which I noticed later on:
>>
>> ,---
>> gaara:~# /etc/init.d/rpcbind start
>> Starting rpcbind daemon....
>> gaara:~# ps axuOp|egrep '(^USER|[r]pcbind)'
>> USER       PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COM=
MAND
>> root     23424  0.0  0.0  18768   704 ?        Ss   13:53   0:00 /sb=
in/rpcbind -w
>> gaara:~# /etc/init.d/rpcbind stop
>> Stopping rpcbind daemon....
>> gaara:~# dd if=3D/dev/urandom of=3D/tmp/rpcbind.xdr bs=3D1024 count=3D=
1
>> 1+0 records in
>> 1+0 records out
>> 1024 bytes (1,0 kB) copied, 0,000861307 s, 1,2 MB/s
>> gaara:~# /etc/init.d/rpcbind start
>> Starting rpcbind daemon....
>> gaara:~# ps axuOp|egrep '(^USER|[r]pcbind)'
>> USER       PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COM=
MAND
>> root     23440  0.0  0.0 4008972  772 ?        Ss   13:54   0:00 /sb=
in/rpcbind -w
>> `---
>>
>> The first start is a normal clean invokation, the second one is usin=
g
>> the crafted file. See how it has allocated almost 4 GiB. Disregard t=
hough,
>> me running all this as root, a user would be able to craft those fil=
es as
>> long as they were not already in /tmp.
>>
>> thanks,
>> guillem
>
> I'm sending this bug report to the linux-nfs mailing list.
>
> The original bug report is at http://bugs.debian.org/583435

Would /var/run (or a subdirectory of it) be a better choice than /tmp ?

Bug#583435: rpcbind: Insecure handling of state files

[Guillem Jover]

Hi!

On Thu, 2010-06-03 at 16:07:50 -0400, Chuck Lever wrote:
> On 06/ 2/10 07:25 AM, An=C3=ADbal Monsalve Salazar wrote:
> > On Tue, Jun 01, 2010 at 02:09:07PM +0200, Guillem Jover wrote:
> > > On Thu, 2010-05-27 at 19:09:08 +0200, Guillem Jover wrote:
> > > > Package: rpcbind
> > > > Version: 0.2.0-4
> > > > Severity: serious
> > > > Tags: security
> > >
> > > > The rpcbind daemon, which runs as root, uses /tmp/portmap.xdr and
> > > > /tmp/rpcbind.xdr for doing warm starts as what seems to be a way =
to
> > > > preserve state between invokations. It parses (through libtirpc) =
and
> > > > removes them on start. It creates them before exiting.
> > > >
> > > > So first off, *any* user can craft those two files before the dae=
mon
> > > > has started for the first time, which the daemon will parse. This
> > > > might be ok, depending on the checks done on parse, I'd still be =
very
> > > > wary of letting a user be able to craft such files at will.
> > >
> > > It seems to be doing no checks whatsoever. A simple test I performe=
d at
> > > the time of filing this report, but didn't seem to have any obvious
> > > consequence, shows this which I noticed later on:

> > The original bug report is at http://bugs.debian.org/583435

I'm adding here part of the initial mail that I trimmed when replying
to myself:

,---
The second problem is that those files get created by the daemon on
shutdown, and they *do* follow symlinks. So a user can drop two
symlinks
there while the daemon is running and overwrite any file on the file
system on shutdown.

The fix would consist of passing to configure something like
=E2=80=9C--with-statedir=3D/var/cache/rpcbind=E2=80=9D, and make sure the=
 daemon creates
such directory if missing on exit in src/warmstart.c:write_struct(),
which it does not seem to be doing currently.

In addition it would be wise to notify upstream to change the default
statedir to something else than /tmp.
`---

> Would /var/run (or a subdirectory of it) be a better choice than /tmp ?

/var/run might not be preserved across reboots, but regardless of that I
think /var/cache is a better fit, it's internal state, but it's used
to speed up start up time, and can be removed w/o ill effects.

regards,
guillem

Re: Bug#583435: rpcbind: Insecure handling of state files

[Chuck Lever]

On 06/ 3/10 04:27 PM, Guillem Jover wrote:
> Hi!
>
> On Thu, 2010-06-03 at 16:07:50 -0400, Chuck Lever wrote:
>> On 06/ 2/10 07:25 AM, An=C3=ADbal Monsalve Salazar wrote:
>>> On Tue, Jun 01, 2010 at 02:09:07PM +0200, Guillem Jover wrote:
>>>> On Thu, 2010-05-27 at 19:09:08 +0200, Guillem Jover wrote:
>>>>> Package: rpcbind
>>>>> Version: 0.2.0-4
>>>>> Severity: serious
>>>>> Tags: security
>>>>
>>>>> The rpcbind daemon, which runs as root, uses /tmp/portmap.xdr and
>>>>> /tmp/rpcbind.xdr for doing warm starts as what seems to be a way =
to
>>>>> preserve state between invokations. It parses (through libtirpc) =
and
>>>>> removes them on start. It creates them before exiting.
>>>>>
>>>>> So first off, *any* user can craft those two files before the dae=
mon
>>>>> has started for the first time, which the daemon will parse. This
>>>>> might be ok, depending on the checks done on parse, I'd still be =
very
>>>>> wary of letting a user be able to craft such files at will.
>>>>
>>>> It seems to be doing no checks whatsoever. A simple test I perform=
ed at
>>>> the time of filing this report, but didn't seem to have any obviou=
s
>>>> consequence, shows this which I noticed later on:
>
>>> The original bug report is at http://bugs.debian.org/583435
>
> I'm adding here part of the initial mail that I trimmed when replying
> to myself:
>
> ,---
> The second problem is that those files get created by the daemon on
> shutdown, and they *do* follow symlinks. So a user can drop two
> symlinks
> there while the daemon is running and overwrite any file on the file
> system on shutdown.
>
> The fix would consist of passing to configure something like
> =E2=80=9C--with-statedir=3D/var/cache/rpcbind=E2=80=9D, and make sure=
 the daemon creates
> such directory if missing on exit in src/warmstart.c:write_struct(),
> which it does not seem to be doing currently.
>
> In addition it would be wise to notify upstream to change the default
> statedir to something else than /tmp.
> `---

Agree changing the upstream default is a good idea.

Generally, that kind of directory is created as part of installation=20
(like, by rpm --install) rather than by the daemon itself.

>> Would /var/run (or a subdirectory of it) be a better choice than /tm=
p ?
>
> /var/run might not be preserved across reboots, but regardless of tha=
t I
> think /var/cache is a better fit, it's internal state, but it's used
> to speed up start up time, and can be removed w/o ill effects.

No, it's not intended to speed start up.

The cache files aren't really supposed to be retained over a reboot.=20
After a system restart, all of the RPC services will restart and=20
register themselves again.  If just rpcbind restarts, all that=20
registration state is lost, so that's the point of saving it in a file.

I don't have a preference wrt /var/run or /var/cache.

Re: Bug#583435: rpcbind: Insecure handling of state files

[Guillem Jover]

On Thu, 2010-06-03 at 16:34:01 -0400, Chuck Lever wrote:
> On 06/ 3/10 04:27 PM, Guillem Jover wrote:
> >The second problem is that those files get created by the daemon on
> >shutdown, and they *do* follow symlinks. So a user can drop two
> >symlinks
> >there while the daemon is running and overwrite any file on the file
> >system on shutdown.
> >
> >The fix would consist of passing to configure something like
> >=E2=80=9C--with-statedir=3D/var/cache/rpcbind=E2=80=9D, and make sur=
e the daemon creates
> >such directory if missing on exit in src/warmstart.c:write_struct(),
> >which it does not seem to be doing currently.
> >
> >In addition it would be wise to notify upstream to change the defaul=
t
> >statedir to something else than /tmp.
>=20
> Agree changing the upstream default is a good idea.
>=20
> Generally, that kind of directory is created as part of installation
> (like, by rpm --install) rather than by the daemon itself.

At least for /var/run I think it's common for systems to mount it
as tmpfs, so the directories might not be there on boot. But those can
always be created by the init script (or equivalent), it might be a
problem if run from inetd though.

> >>Would /var/run (or a subdirectory of it) be a better choice than /t=
mp ?
> >
> >/var/run might not be preserved across reboots, but regardless of th=
at I
> >think /var/cache is a better fit, it's internal state, but it's used
> >to speed up start up time, and can be removed w/o ill effects.
>=20
> No, it's not intended to speed start up.
>=20
> The cache files aren't really supposed to be retained over a reboot.
> After a system restart, all of the RPC services will restart and
> register themselves again.  If just rpcbind restarts, all that
> registration state is lost, so that's the point of saving it in a
> file.

Ah, yeah that makes more sense! More so given the configure option, I
should have written "AFAIS" or something like that. :)

> I don't have a preference wrt /var/run or /var/cache.

So given that this is actually run time state, /var/run seems more
appropriate, indeed.

regards,
guillem

Re: Bug#583435: rpcbind: Insecure handling of state files

[Chuck Lever]

On 06/ 3/10 05:07 PM, Guillem Jover wrote:
> On Thu, 2010-06-03 at 16:34:01 -0400, Chuck Lever wrote:
>> On 06/ 3/10 04:27 PM, Guillem Jover wrote:
>>> The second problem is that those files get created by the daemon on
>>> shutdown, and they *do* follow symlinks. So a user can drop two
>>> symlinks
>>> there while the daemon is running and overwrite any file on the fil=
e
>>> system on shutdown.
>>>
>>> The fix would consist of passing to configure something like
>>> =E2=80=9C--with-statedir=3D/var/cache/rpcbind=E2=80=9D, and make su=
re the daemon creates
>>> such directory if missing on exit in src/warmstart.c:write_struct()=
,
>>> which it does not seem to be doing currently.
>>>
>>> In addition it would be wise to notify upstream to change the defau=
lt
>>> statedir to something else than /tmp.
>>
>> Agree changing the upstream default is a good idea.
>>
>> Generally, that kind of directory is created as part of installation
>> (like, by rpm --install) rather than by the daemon itself.
>
> At least for /var/run I think it's common for systems to mount it
> as tmpfs, so the directories might not be there on boot. But those ca=
n
> always be created by the init script (or equivalent), it might be a
> problem if run from inetd though.

Sure, that makes sense.  Having the daemon create the directory also=20
means there are fewer ways distributors can get this wrong.

>>>> Would /var/run (or a subdirectory of it) be a better choice than /=
tmp ?
>>>
>>> /var/run might not be preserved across reboots, but regardless of t=
hat I
>>> think /var/cache is a better fit, it's internal state, but it's use=
d
>>> to speed up start up time, and can be removed w/o ill effects.
>>
>> No, it's not intended to speed start up.
>>
>> The cache files aren't really supposed to be retained over a reboot.
>> After a system restart, all of the RPC services will restart and
>> register themselves again.  If just rpcbind restarts, all that
>> registration state is lost, so that's the point of saving it in a
>> file.
>
> Ah, yeah that makes more sense! More so given the configure option, I
> should have written "AFAIS" or something like that. :)
>
>> I don't have a preference wrt /var/run or /var/cache.
>
> So given that this is actually run time state, /var/run seems more
> appropriate, indeed.