Markus Roberts <[email protected]> writes:
> Added a random suffex to the temporary file name and verify that the name is
> not in use (retry on collison). No additional tests added as the lifetime
> of the temporary file is bound by the routine, making spec driven tests
> impraticable.
This has a security flaw, I fear to say.
[...]
> use_temporary_file = (content.length != 0)
> - path = self[:path]
> - path += ".puppettmp" if use_temporary_file
> + if use_temporary_file
> + path = "#{self[:path]}.puppettmp_#{rand(10000)}" until
> !File.exists?(path)
> + else
> + path = self[:path]
> + end
>
> mode = self.should(:mode) # might be nil
> umask = mode ? 000 : 022
There is a race between the check and the use of the path; you can close that
by changing the creation of the file below to fail when the target exists.
Without that it is possible to place a symlink under puppet if a file is
written in a directory I have access to, causing it to write through that and
at least truncate existing files, if not more.
The File.open call just below should be amended to include File::EXCL in the
call; that conflicts with the use of O_TRUNC in the current call, though, so
you would need to restructure the logic to handle the existing vs non-existing
files sensibly.
Further, you have a data-loss risk further down, where File.rename is used to
overwrite an existing file in the 'use_temporary_file' case; you need to use
fdatasync or fsync[1] on the file before the rename. In this case, inside the
block attached to File.open is the appropriate place for f.fsync.
Without that you run the risk that the content of the file will not be
recorded, but the rename will be, and you end up with a zero length file
replacing the old version.
Regards,
Daniel
Footnotes:
[1] Through IO.fsync, which I /think/ is documented to do the right thing in
flushing ruby internal IO buffers as well as OS buffers, but you should
probably check.
--
✣ Daniel Pittman ✉ [email protected] ☎ +61 401 155 707
♽ made with 100 percent post-consumer electrons
Looking for work? Love Perl? In Melbourne, Australia? Let me know.
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, visit this group at
http://groups.google.com/group/puppet-dev?hl=en
-~----------~----~----~----~------~----~------~--~---
