Code review comment for lp:~brian-murray/apport/bug-1500541

Revision history for this message
Martin Pitt (pitti) wrote :

Making up new file names on the fly like this is dangerous. It's susceptible to symlink attacks in world-write directories such as /tmp/ or /var/crash/, or it might overwrite an already existing file.

The check whether to create an output file could be moved to the top, and apport-retrace could then (and only then) open the file as 'r+b', and seek() to the start before writing.

Alternatively it could open the .out file as 'xb' for exclusive creation, which avoids the above two problems. This isn't available for python 2 yet though; for the deb packages that doesn't matter, but we must pay attention to the data center retracers. Mine don't use output files, so it'd be fine for me. But I still don't like the creation of arbitrary output file names, that's not very predictable.

Thanks!

review: Needs Fixing

« Back to merge proposal