On Sun, 2008-08-24 at 22:05 +0400, Dmitry E. Oboukhov wrote:
> Package: datafreedom-perl
> Severity: grave

No, that is just plain wrong, sorry.

> Hi, maintainer!

(and I do so hate unnecessary exclamation marks)

> This message about the error concerns a few packages at once. I've
> tested all the packages (for Lenny) on my Debian mirror. All scripts
> of packages (marked as executable) were tested.
>
> In some packages I've discovered scripts with errors which may be used
> by a user for damaging important system files or user's files.

Dmitry - the discussion on -devel did note the risks of false positives
and I would have expected that you would have taken more care before
starting to file bugs.

> For example if a script uses in its work a temp file which is created
> in /tmp directory, then every user can create symlink with the same
> name in this directory in order to destroy or rewrite some system
> or user file. Symlink attack may also lead not only to the data
> desctruction but to denial of service as well.

Not when the use of /tmp is a *suggestion in a manpage* which just
happens to be generated from POD content that is commonly embedded
within perl scripts.

=head1
A more complex example using 'zenity' - a Gnome dialog generator.

$ pilot-qof -x data.xml --invoice-city -t 2006-11-08 | dfxml-invoice -
> /tmp/zenity
zenity --text-info --title="2006-11-08" --filename=/tmp/zenity
--width=500 --height=300
=cut

The program does not create this file, it does not rely on this file, it
does not require any specific filename in /tmp and it does not write any
data to /tmp unless the USER specifically pipes the STDOUT to a file and
happens to use /tmp for that file.

If the user is dumb enough to pipe the output to a file that is a
symlink to something more important *AND* which has sufficient
permissions to be a problem, then that is not the fault of the package.
It is an example, nothing more.

If you have filed *grave* bugs against every package that includes
embedded documentation or manpage content outlining the possibility of
piping STDOUT to a file that might happen to be somewhere in /tmp then
you have made a huge mis-calculation.

The manpage example is deliberately simplified for clarity - yes, it
could have been made a lot longer by exporting a generated filename to a
variable and reading that variable into the command line to zenity but
there really was no point. /tmp was used because files in /tmp will be
cleared out at reboot so that I didn't have to remove the file later
(when it might be useful to run the zenity process a few times). Why
make the example unnecessarily complicated? All it needed to show was
that the redirect needs to create a file which zenity can then read.

> This list is created with the help of script. This list is sorted by
> hand. Howewer in some cases mistake is possible.

You're not kidding.

> Please, Be understanding to possible mistakes.

Knowing that mistakes were likely makes it all the more annoying that
you didn't file a LIST of possible bugs to -devel, you just went ahead.
It really wasn't a good idea, that one.

> I set Severity into grave for this bug. The table of discovered
> problems is below.

That is just the WRONG way to do a mass bug filing, IMHO. With a release
pending and all these bugs supposedly *grave*, the right way to do it
would have been to use the mass bug filing support in devscripts to send
a list of affected packages and maintainers to -devel BEFORE filing any
bugs such as to allow maintainers to identify more false positives and
prevent needless churn in the BTS.

> Discussion of this bug you can see in debian-devel@:
> http://lists.debian.org/debian-devel/2008/08/msg00271.html

Thanks for the entirely needless hassle of having to close #496429
immediately after it was filed instead of the simple courtesy of posting
your intention along with the complete list of packages and maintainers.

--


Neil Williams
=============
http://www.data-freedom.org/
http://www.nosoftwarepatents.com/
http://www.linux.codehelp.co.uk/

On Sun, Aug 24, 2008 at 08:28:32PM +0100, Neil Williams wrote:
> > For example if a script uses in its work a temp file which is created
> > in /tmp directory, then every user can create symlink with the same
> > name in this directory in order to destroy or rewrite some system
> > or user file. Symlink attack may also lead not only to the data
> > desctruction but to denial of service as well.

> Not when the use of /tmp is a *suggestion in a manpage* which just
> happens to be generated from POD content that is commonly embedded
> within perl scripts.

> =head1
> A more complex example using 'zenity' - a Gnome dialog generator.

> $ pilot-qof -x data.xml --invoice-city -t 2006-11-08 | dfxml-invoice -
> > /tmp/zenity
> zenity --text-info --title="2006-11-08" --filename=/tmp/zenity
> --width=500 --height=300
> =cut

> The program does not create this file, it does not rely on this file, it
> does not require any specific filename in /tmp and it does not write any
> data to /tmp unless the USER specifically pipes the STDOUT to a file and
> happens to use /tmp for that file.

Yes, this is definitely another false positive, which is very unfortunate.

However,

> If the user is dumb enough to pipe the output to a file that is a
> symlink to something more important *AND* which has sufficient
> permissions to be a problem, then that is not the fault of the package.
> It is an example, nothing more.

The example *is* wrong - the example given is never safe to run, because the
only way to verify beforehand that /tmp/zenity is not a symlink to something
more important is by first explicitly *creating* your file funder /tmp
(non-destructively), then check that it's not a symlink, and *then* run
pilot-qof. Otherwise, there is always a race condition here between
checking for non-existence, and outputting to the file, tha is exploitable
for some ill purpose.

--
Steve Langasek Give me a lever long enough and a Free OS
Debian Developer to set it on, and I can move the world.
Ubuntu Developer http://www.debian.org/
slangasek@ubuntu.com vorlon@debian.org


--
To UNSUBSCRIBE, email to debian-devel-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org

On Sun, 2008-08-24 at 13:30 -0700, Steve Langasek wrote:
> On Sun, Aug 24, 2008 at 08:28:32PM +0100, Neil Williams wrote:
> > =head1
> > A more complex example using 'zenity' - a Gnome dialog generator.
>
> > $ pilot-qof -x data.xml --invoice-city -t 2006-11-08 | dfxml-invoice -
> > > /tmp/zenity
> > zenity --text-info --title="2006-11-08" --filename=/tmp/zenity
> > --width=500 --height=300
> > =cut
> The example *is* wrong - the example given is never safe to run, because the
> only way to verify beforehand that /tmp/zenity is not a symlink to something
> more important is by first explicitly *creating* your file funder /tmp
> (non-destructively), then check that it's not a symlink, and *then* run
> pilot-qof. Otherwise, there is always a race condition here between
> checking for non-existence, and outputting to the file, tha is exploitable
> for some ill purpose.

By whom and how? What kind of attacker is going to assume that a
particular user not only has the package installed but has also chosen
to use this specific one of a couple of dozen examples available in the
doc package that goes alongside this one package, to create a particular
file which is itself part of a process that even the most regular user
operates once a month at most *and* that the user has not implemented
the example in some other script that does various other checks? The
number of users who need this particularly specialised example is very
small (which is why it is not implemented directly within
dfxml-invoice). An attacker would be insane to select this example as a
vehicle. The whole point of the package is to support the use of the
data in a wider variety of scripts, written by users for themselves. The
example exists to show what can be done, not what every user is expected
to do.

Next release could drop the temporary file from the example completely -
zenity does support '-', just like dfxml-invoice:

$ pilot-qof -x data.xml --invoice-city -t 2006-11-08 | dfxml-invoice -
| zenity --text-info --title="2006-11-08" -

However, this is sub-optimal for two reasons:
1. It is wasteful - the display of the data via zenity may need to be
repeated so re-calculating the data is a waste
2. Unnecessarily complicated for documentation (the need for ' is,
IMHO, an indication that the command is too long).

I'm still not sure that this is an actual problem.

Yes, a race condition could happen and yes, there could be all sorts of
complicated ways of handling temp files and passing back the name of the
file but examples have to be simple and clear, not obfuscated by
problems unrelated to the nature of the example itself.

This is just a simple redirect and is not even part of the program, any
number of non-packaged scripts use redirects all over the OS and users
really do need to take some accountability for what might happen if they
copy examples literally. How many users have '> /tmp/foo' in their own
scripts? (or > /home/user/foo which can be just as problematic and is
completely pointless in documentation). /tmp is the natural place for
temporary data - files that the user might need a few times over a
finite period of time but which do not need to be kept around
indefinitely. Constantly renaming files that the user may want to use a
few times makes it pointless using the temporary file and causes
unnecessary recalculation of the data.

Do we want to sanitize every example and piece of documentation? Is it
reasonable to obfuscate every example of a redirect with dire security
warnings?

Before I tested 'zenity --foo bar -', I was just considering replacing
'/tmp/zenity' with '/path/to/tmp/file' but do we really want to go to
such lengths?

Where should the lines be drawn between clear documentation, overly
cautious security and user-written scripts?

--


Neil Williams
=============
http://www.data-freedom.org/
http://www.nosoftwarepatents.com/
http://www.linux.codehelp.co.uk/

[Neil Williams]
> $ pilot-qof -x data.xml --invoice-city -t 2006-11-08 | dfxml-invoice -
> | zenity --text-info --title="2006-11-08" -
>
> 2. Unnecessarily complicated for documentation (the need for ' is,
> IMHO, an indication that the command is too long).

Not to disagree with your real thesis, but there is no need for here.

$ pilot-qof -x data.xml --invoice-city -t 2006-11-08 |
dfxml-invoice - | zenity --text-info --title="2006-11-08" -

Peter, on behalf of the Society for the Preservation of Useful Details
of Shell Syntax
--
Peter Samuelson | org-tld!p12n!peter | http://p12n.org/


--
To UNSUBSCRIBE, email to debian-devel-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org

On Sun, Aug 24, 2008 at 06:44:57PM -0700, Russ Allbery wrote:
> Steve Langasek <vorlon@debian.org> writes:

> > The example *is* wrong - the example given is never safe to run, because
> > the only way to verify beforehand that /tmp/zenity is not a symlink to
> > something more important is by first explicitly *creating* your file
> > funder /tmp (non-destructively), then check that it's not a symlink, and
> > *then* run pilot-qof.

> I dunno, I'd feel quite comfortable running that command on my personal
> laptop, which has no other users and no remote login access. /tmp file
> vulnerabilities are only vulnerabilities on multiuser systems. We don't
> know for *packages* whether they'll be installed on multiuser systems, so
> of course we have to fix them regardless, but in examples I think it's
> often reasonable to be sloppier.

Sloppy examples are not a high priority, but I think it's still a bug,
because we should try to set better examples. All kinds of people follow
examples, including some who go on to write scripts that end up packaged
later and run by our users...

--
Steve Langasek Give me a lever long enough and a Free OS
Debian Developer to set it on, and I can move the world.
Ubuntu Developer http://www.debian.org/
slangasek@ubuntu.com vorlon@debian.org


--
To UNSUBSCRIBE, email to debian-devel-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org

NW> An attacker would be insane to select this example as a
NW> vehicle.

Attacker can use many ways (all variants from this list, for ex), one of
its can work. Why you think that this variant is not work?

--

. '`. Dmitry E. Oboukhov
: :’ : unera@debian.org
`. `~’ GPGKey: 1024D / F8E26537 2006-11-21
`- 1B23 D4F8 8EC0 D902 0555 E438 AB8C 00CF F8E2 6537

On Mon, 2008-08-25 at 11:57 +0400, Dmitry E. Oboukhov wrote:
> NW> An attacker would be insane to select this example as a
> NW> vehicle.
>
> Attacker can use many ways (all variants from this list, for ex), one of
> its can work. Why you think that this variant is not work?

Because it is in the documentation, not the script. Didn't you read the
reply? It is not a route of attack, it is AN EXAMPLE in the
documentation!

What kind of attacker is going to assume that a particular user not only
has the package installed but has also chosen to use this specific one
of a couple of dozen examples available in the doc package that goes
alongside this one package, to create a particular file which is itself
part of a process that even the most regular user operates once a month
at most *and* that the user has not implemented the example in some
other script that does various other checks? The number of users who
need this particularly specialised example is very small (which is why
it is not implemented directly within dfxml-invoice). An attacker would
be insane to select this example as a vehicle. The whole point of the
package is to support the use of the data in a wider variety of scripts,
written by users for themselves. The example exists to show what can be
done, not what every user is expected to do.

Stop blindly applying your (broken) logic and *think* about the bugs
that you are filing, Dmitry.

The example is only a problem if a user implements the documentation
without thinking *and* uses it on a multi-user system. The idea that
this is a grave security issue is ludicrous - you might as well assert
that all interpreters must be withdrawn because they can be used in
security attacks. I know, let's ban all means of file storage because
files can be used in attacks - even better, ban all methods of executing
the content of any file, whether in RAM or in storage. No, wait, that's
insane.

Blind application of logic will end up with a ban on oxygen because it
is fatal to all life on earth. The logic is correct, even if all human
disease is cured, oxygen will cause the death of every human on earth
(over a sufficient period of time) due to the role of oxygen radicals in
the ageing process, it still doesn't mean it is correct to apply the
result of that logic.

If you extrapolate pure logic far enough, every possibility becomes
probable. If you allow enough time, every probable risk becomes a
certainty. You can't use blind logic to assert risk, the logic must be
tempered within the bounds of what is reasonable, what is likely and
what is realistic.

IMHO, the risk of bugs (including quite a few security ones) due to
time_t overflowing in 2038 is far more likely than a real security issue
from #496429. That is why pilot-qof (via the QOF library) has 64bit time
handling on all platforms but this rather innocent example. It is *not*
reasonable to say that even 64bit time is insufficient and that all time
must be 128bit or 1024bit or gigabit etc. 64bit already includes enough
seconds to encompass a couple of dozen times the (estimated) age of the
universe, 32bit time_t comes to an abrupt end in 2038. This about
proportionality, reasonable risk and a realistic assessment of security
implications.

Security measures *must* be proportional to the actual risk. That is the
main the problem with this mass bug filing, the security implications
are not equal across all packages which have had RC bugs filed against
them.

Just because your (broken) script has found a pattern, does NOT mean
that the pattern is a security risk - it is incumbent on YOU to check
the output of each individual pattern match in each individual package
and make a sane determination of the actual risk of that pattern in that
package. The bug should then detail the *specific risk* within that
package, an assessment of why that specific risk in that specific
package is actually a problem for that specific package within the
normal behaviour of that package. With a release so close, filing RC
bugs without due care is, as Steve described in #496386, simply not good
enough:

"This is far below the quality I expect from a mass bug filing that's
been reviewed by debian-devel. Mass bugfilings at RC severity need to be
held to a much higher standard than this, particularly when we're in the
middle of a release freeze."

I take Steve's point that the example could be improved (although I'm
not entirely sure what form the change should take so as to not
obfuscate the example in security warnings). Such a request for an
improved example is NOT a security issue or an RC bug! There is no way
that this example should delay the release of Lenny.

I'm tempted to close all other bugs filed during this mass bug filing
against packages that I use without any further comment or fix or
explanation, simply because the mass bug filing itself is so
unreasonable. Why should I invest the time trying to determine if the
bug is an actual risk when, so far, I have found all instances to have
been filed without sufficient care or diligence?

If the submitter doesn't care about checking if the bug is real or
imagined, why should I believe that the risk is real? Your lack of due
diligence has invalidated the entire mass bug filing attempt and
undermined every bug you have filed for this supposed problem.

IMHO, the only acceptable response to this inept bug filing is for you
to immediately close all the bugs you have filed under this mass bug
filing so far, go back to the beginning and take the time to make an
individual assessment of the individual risk of each package and script
that your process has so far identified. Then, in the accepted manner,
using '/usr/bin/mass-bug' from devscripts, you post again to
debian-devel, including a detailed list of packages, maintainer and
uploaders (mass-bug does this for you) against which you PROPOSE to file
bugs and give time for responses. According to the responses, you
optimise the process further and repeat until there is a consensus that
the methodology has done everything reasonable to exclude false
positives (including not scanning patterns within embedded documentation
like POD). Then, you individually file each bug with a clear description
of the problem as it applies to that specific package and you set the
severity and tags appropriately for each individual bug. During this
process, you may well generate a patch that fixes the problem and you
can then attach that to the bug report.

This is what I have been doing for the long term mass bug filing for
crossbuilding support that I started in November 2007 and which will
continue until probably just before the release of Lenny+1.

http://bugs.debian.org/cgi-bin/pkgreport.cgi?usertag=codehelp@debian.org

Status
* 59 Outstanding
* 2 Pending Upload
* 39 Resolved

I don't care that it takes longer, it gets better results and is the
right way to do a mass bug filing.

--


Neil Williams
=============
http://www.data-freedom.org/
http://www.nosoftwarepatents.com/
http://www.linux.codehelp.co.uk/

NW>>> An attacker would be insane to select this example as a
NW>>> vehicle.
NW>>
NW>> Attacker can use many ways (all variants from this list, for ex), one of
NW>> its can work. Why you think that this variant is not work?

NW> Because it is in the documentation, not the script. Didn't you read the
NW> reply? It is not a route of attack, it is AN EXAMPLE in the
NW> documentation!
This script marked as executable.
User can start its.

if it is an example, please chmod a-x to it

--

. '`. Dmitry E. Oboukhov
: :’ : unera@debian.org
`. `~’ GPGKey: 1024D / F8E26537 2006-11-21
`- 1B23 D4F8 8EC0 D902 0555 E438 AB8C 00CF F8E2 6537

On Mon, 2008-08-25 at 14:17 +0400, Dmitry E. Oboukhov wrote:
> NW>>> An attacker would be insane to select this example as a
> NW>>> vehicle.
> NW>>
> NW>> Attacker can use many ways (all variants from this list, for ex), one of
> NW>> its can work. Why you think that this variant is not work?
>
> NW> Because it is in the documentation, not the script. Didn't you read the
> NW> reply? It is not a route of attack, it is AN EXAMPLE in the
> NW> documentation!

> This script marked as executable.
> User can start its.

Dimtry, please read the replies I've already sent. This is embedded
documentation within an executable script. The documentation is stripped
from the script by the perl interpreter before compilation. This is a
standard method of combining documentation into perl scripts called POD
and it is not a security risk!!!

Think of POD as if it was /* foo */ in a C source file. It is excluded
from the compilation of the script by the perl interpreter, it is
available to perldoc and pod2man scripts to generate documentation. It
is NOT executable.

> if it is an example, please chmod a-x to it

It is an example in POD documentation. POD documentation is included in
the script but it is NEVER executed.

=head1

foo

=cut

Anything between the POD markers is NOT executable. There is absolutely
NO way that the user can execute the contents of a POD block without
copying and pasting to their own script.

--


Neil Williams
=============
http://www.data-freedom.org/
http://www.nosoftwarepatents.com/
http://www.linux.codehelp.co.uk/