Re: process-deb-once-with-python review

[Niels Thykier <niels@thykier.net>]

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