On Mon, Dec 20, 2004 at 01:24:03AM +0100, Jeroen van Wolffelaar wrote:
>
> Argh, after looking again, I still stand by my initial assassment, I was
> misleaded by the theory that the logic was bogus. The key point is:
>
> | if (not -d "$LINTIAN_LAB" or ($lab_mode eq 'temporary'))
> | mkdir($LINTIAN_LAB,0777) or fail("cannot create lab directory $LINTIAN_LAB")
>
> And, this is correct. If $lab_mode is not temporarily, a lab location
> was specifically given to lintian, and we should assume that the invoker
> of lintian in that case knows what he does. In all other cases, i.e.,
> lab_mode equals temporary, the condition in the if is true (note the
> 'or'), and the lab dir is unconditionally tried to be made, which fails
> if it already exists.
_However_ if it does not exist the lab is created, if the user has a
"insecure" umask (002) you will end up being prey to symlink attacks due
to a race condition check this (when setting up the lab):
_touch("$LINTIAN_LAB/info/binary-packages")
or fail("cannot create binary package list");
_touch("$LINTIAN_LAB/info/source-packages")
or fail("cannot create source package list");
_touch("$LINTIAN_LAB/info/udeb-packages")
or fail("cannot create udeb package list");
And
sub _touch {
open(T,">$_[0]") or return 0;
close(T) or return 0;
return 1;
}
Image the following race condition:
1.- User A in group B with umask 002 runs 'lintian package.deb'
2.- The following structure is created:
[first steps of Lab::setup]
/tmp/lintian-lab.8624/
|-- binary
|-- info
|-- source
`-- udeb
3.- User R in group B symlinks the following files to files that belong to
user A:
|-- info
| |-- binary-packages
| |-- source-packages
| `-- udeb-packages
4.- [final steps of Lab::setup]
_touch is executed and the files that A's files where B
symlinked the previous files to are emptied because of the race condition.
So what you have here is a race condition because of temporary
files. Granted, this only happens when you have a lax umask, but it could
be prevented either by using a proper function to create temporary
directories (tempdir() will set them up mode 700) or by restricting the
umask to 0700 when creating the temporary directories in Lab::setup().
I did not properly assess the issue in the initial report, but I still
believe that Lintian has a security bug which introduces a hole under some
conditions and that could be easily fixed.
Regards
Javier
PS: Yes, version was 1.23.3, blame it on opening up multiple bug reports at
1 am
PPS: I already informed the security team and was told to follow this bugs
up on the BTS
Attachment:
signature.asc
Description: Digital signature