Re: [Bug-wget] Wget - acess list bypass / race condition PoC

[Dawid Golunski]

CVE-ID team, could you please let us know a CVE-ID number we could use
for the GNU Wget vulnerability described in this thread ?

Thanks

On Wed, Aug 17, 2016 at 7:03 PM, Dawid Golunski <address@hidden> wrote:
> Hi Tim,
>
> Thanks for this.  The filename generation with temporary name in it looks 
> good.
> As for the credit, I discovered this vulnerability and passed it on to
> VeriSign, so I'd appreciate it if you could add 1 more credit in the
> patch / bug announcement as:
>
> Discovered by: Dawid Golunski (http://legalhackers.com)
>
> As for the CVEID, I think we should email this to address@hidden
>
> Thanks.
>
>
>
> On Wed, Aug 17, 2016 at 4:48 PM, Tim Rühsen <address@hidden> wrote:
>> Please review / test this patch.
>>
>> Please check the 'Reported-by' in the commit message and if you got a CVE
>> number, please report for inclusion into the commit message (and/or the 
>> code).
>>
>> Regards, Tim
>>
>> On Mittwoch, 17. August 2016 10:40:35 CEST Dawid Golunski wrote:
>>> Random file name + .part extension on temporary files would already be
>>> good improvement (even if still stored within the same directory) and
>>> help prevent the exploitation.
>>>
>>> Thanks.
>>>
>>> On Wed, Aug 17, 2016 at 9:22 AM, Tim Rühsen <address@hidden> wrote:
>>> > On Mittwoch, 17. August 2016 13:37:33 CEST Ander Juaristi wrote:
>>> >> I was thinking we could rename php extensions to phps, but it's all the
>>> >> same thing in the end, and even better, since the former applies to any
>>> >> kind of file and I've seen some broken servers that actually run phps
>>> >> files.>>
>>> >> So, this is what I would do:
>>> >>     1. Write temporary files with 600 perms, and make sure they're owned
>>> >>
>>> >> by the running user and group. qmail goes even further [1] by not
>>> >> letting root run, but I would not do that here.
>>> >>
>>> >>     2. Use mkostemp() to generate a unique filename and give it a
>>> >>
>>> >> harmless extension (like Mozilla's .part). We already have unique_name()
>>> >> in utils.c, altough it returns the file name untouched if it does not
>>> >> exist. We should do some research on whether we could reuse parts of it.
>>> >
>>> > Giuseppe and I  have a working patch that is basically like this. We are
>>> > still discussing the details (mkstemp extension, fixed extension, both or
>>> > maybe a mkdtemp directory where we put all the temp files).
>>> >
>>> > As soon as we agree, we'll post the patch here for further
>>> > discussion/review.>
>>> >>     3. Place them in /tmp, or even better, in ~/.wget-tempfiles, or
>>> >>
>>> >> something like that.
>>> >
>>> > /tmp often is on a separate filesystem (e.g. RAM disk) with limited space.
>>> > This could open another (DOS) attack vector.
>>> >
>>> > You do not always have a home directory when running Wget.
>>> >
>>> >> There's a patch by Tim somewhere in this list that already does 1 (but
>>> >> please, remove the braces ;D).
>>> >>
>>> >> It also comes to my mind, instead of writing each temp file to its own
>>> >> file, we could put them all in the same file (with O_APPEND). But a) we
>>> >> need a way to tell them apart later, and b) it may cause problems in
>>> >> NFS, according to open(2).
>>> >>
>>> >> [1] http://cr.yp.to/qmail/guarantee.html
>>> >>
>>> >> On 15/08/16 18:31, Tim Rühsen wrote:
>>> >> > On Montag, 15. August 2016 10:02:55 CEST moparisthebest wrote:
>>> >> >> Hello,
>>> >> >>
>>> >> >> I find it extremely hard to call this a wget vulnerability when SO
>>> >> >> many
>>> >> >> other things are wrong with that 'vulnerable code' implementation it
>>> >> >> isn't even funny:
>>> >> >>
>>> >> >> 1. The image_importer.php script takes a single argument, why would it
>>> >> >> download with the recursive switch turned on?  Isn't that clearly a
>>> >> >> bug
>>> >> >> in the php script?  Has a php script like this that downloads all
>>> >> >> files
>>> >> >> from a website of a particular extension ever been observed in the
>>> >> >> wild?
>>> >> >>
>>> >> >> 2. A *well* configured server would have a whitelist of .php files it
>>> >> >> will execute, making it immune to this.  A *decently* configured
>>> >> >> server
>>> >> >> would always at a minimum make sure they don't execute code in
>>> >> >> directories with user provided uploads in them.  So it's additionally
>>> >> >> a
>>> >> >> bug in the server configuration. (incidentally every php package I've
>>> >> >> downloaded has at minimum a .htaccess in upload directories to prevent
>>> >> >> this kind of thing with apache)
>>> >> >>
>>> >> >> It seems to me like there has always been plenty of ways to shoot
>>> >> >> yourself in the foot with PHP, and this is just another iteration on a
>>> >> >> theme.
>>> >> >
>>> >> > Hi,
>>> >> >
>>> >> > this is absolutely true and your points were the first things that came
>>> >> > to
>>> >> > my mind when reading the original post.
>>> >> >
>>> >> > But there is also non-obvious wget behavior in creating those (temp)
>>> >> > files
>>> >> > in the filesystem. And there is also a long history of attack vectors
>>> >> > introduced by temp files as well.
>>> >> >
>>> >> > Today the maintainers discussed a few possible fixes, all with pros and
>>> >> > cons. I would like to list them here, in case someone likes to comment:
>>> >> >
>>> >> > 1. Rewrite code to keep temp files in memory.
>>> >> > Too complex, needs a redesign of wget. And has been done for wget2...
>>> >> >
>>> >> > 2. Add a harmless extension to the file names.
>>> >> > Possible name collision with wanted files.
>>> >> > Possible name length issues, have to be worked around.
>>> >> >
>>> >> > 3. Using file mode 0 (no flags at all).
>>> >> > Short vulnerability when changing modes to write/read the data.
>>> >> >
>>> >> > 4. Using O_TMPFILE for open().
>>> >> > Just for Linux, not for every filesystem available.
>>> >> >
>>> >> > 5. Using mkostemp().
>>> >> > Possible name collision with wanted files (which would be unexpectedly
>>> >> > named as *.1 in case of a collision). At least the chance for a
>>> >> > collision
>>> >> > seems very low.
>>> >> >
>>> >> > Any thoughts or other ideas ?
>>> >> >
>>> >> > Regards, Tim
>>
>
>
>
> --
> Regards,
> Dawid Golunski
> http://legalhackers.com



-- 
Regards,
Dawid Golunski
http://legalhackers.com