Re: process-deb-once-with-python review
On 2014-07-09 23:42, Jakub Wilk wrote:
> [...]
>
> How about safepath() from the attachment?
>
>
> [I'll reply to the rest of the mail later.]
>
Thanks, merged.
>
> [...]
>> So far I have not had an issue with owners, despite extracting data.tar, where all files are (usually) owned by root/root.
>
> I looked at the source of the tarfile module. Turns out that TarFile.chmod() is no-op when os.getuid() != 0.
>
>> That said, I guess we should be safe rather than sorry here as well.
>
> Agreed.
>
I changed to code to always reset the mode bits (to 0644/0755) and
uid/gid. I was too lazy to use umask and look up the
username/groupname. In the latter case, I just overwrote the original
values with None, so tarfile is forced to use the uid/gid (like when the
tarball has no username/groupname).
>>> The extract() method is unfortunately quite dumb. It will happily write over existing symlink, or write to an existing FIFO, or...
>>
>> If we ensure the extraction does not escape the unpack root and assert that the target does not exist in any way, I guess this problem would be avoided?
>
> That's right.
>
> --
> Jakub Wilk
>
Thanks for confirming.
With those fixes, I believe the code should now be (security-wise)
ready, but please do correct me if I am wrong here. There are still
some missing bits for merging this into master branch (e.g. emulation of
some of tar(1)'s "quicks" irt to warnings/errors and the fallback).
~Niels
Reply to: