Author hynek
Recipients Arfrever, eric.araujo, ezio.melotti, georg.brandl, hynek, jcea, larry, loewis, mrts, ncoghlan, neologix, petri.lehtinen, pitrou, python-dev, rosslagerwall, schmir, tarek, teamnoir
Date 2012-06-28.11:53:27
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <4FEC45B4.8000004@ox.cx>
In-reply-to <1340882549.88.0.883055659456.issue4489@psf.upfronthosting.co.za>
Content
> I'm not a security guy, but: shouldn't the os.unlink call when it isn't a directory specify follow_symlinks=False? 

os.unlink has no follow_symlinks argument. Imagine what would happen if
you‘d do a os.unlink() on a link and it would just remove the link
destination. :)

> And wouldn't it be safer if the os.rmdir() call also used dir_fd=?

Unfortunately, os.rmdir('.', dir_fd=topfd) doesn’t work. As in the worst
case it could delete only an empty directory, I think it’s fine.

> Additionally, I think you missed some stuff for shutil._use_fd_functions.  Assuming I'm right on both of the above, you should also check:
> * os.listdir in os.supports_dir_fd
> * os.rmdir in os.supports_dir_fd
> * os.stat in os.supports_dir_fd
> * os.stat in os.supports_follow_symlinks
> * os.unlink in os.supports_follow_symlinks

Interestingly, os.listdir is not in os.supports_dir_fd although it works:

False

Will you fix it right away or shall I open a ticket?

> I'd spell that
> _use_fd_functions = ({os.listdir, os.open, os.rmdir, os.stat, os.unlink} < 
>     os.supports_dir_fd and
>     {os.stat, os.unlink} <= os.supports_follow_symlinks)

It would be:

_use_fd_functions = ({os.listdir, os.open, os.stat, os.unlink} <=
                     os.supports_dir_fd and
                     os.stat in os.supports_follow_symlinks)

But currently can’t do.

> Finally, up to you, but I'd be tempted to change the "lstat" "and "fstat" calls to "stat" calls using the relevant parameters.

That's not 3.3 fodder IMHO, feel free to open an enhancement ticket.
History
Date User Action Args
2012-06-28 11:53:28hyneksetrecipients: + hynek, loewis, georg.brandl, jcea, ncoghlan, pitrou, larry, schmir, tarek, ezio.melotti, eric.araujo, Arfrever, mrts, neologix, teamnoir, rosslagerwall, python-dev, petri.lehtinen
2012-06-28 11:53:28hyneklinkissue4489 messages
2012-06-28 11:53:27hynekcreate