Merge lp:~brian-murray/apport/bug-1500541 into lp:apport

Proposed by Brian Murray
Status: Needs review
Proposed branch: lp:~brian-murray/apport/bug-1500541
Merge into: lp:apport
Diff against target: 17 lines (+6/-1)
1 file modified
bin/apport-retrace (+6/-1)
To merge this branch: bzr merge lp:~brian-murray/apport/bug-1500541
Reviewer Review Type Date Requested Status
Martin Pitt (community) Needs Fixing
Review via email: mp+281773@code.launchpad.net

Description of the change

When we initially load the report we only open it for reading, later after retracing we open it for writing and if we can't write to the report apport crashes. This is after a fair bit of work, so it'd be better to write to a new file rather than lose that work.

To post a comment you must log in.
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

Unmerged revisions

3044. By Brian Murray

If we cannot write to the output file, write to a new report rather than crash.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bin/apport-retrace'
2--- bin/apport-retrace 2015-09-24 12:19:28 +0000
3+++ bin/apport-retrace 2016-01-06 18:06:54 +0000
4@@ -402,7 +402,12 @@
5
6 else:
7 if options.output is None:
8- out = open(options.report, 'wb')
9+ try:
10+ out = open(options.report, 'wb')
11+ except PermissionError:
12+ apport.error("Cannot write to %s, writing to %s.retraced instead." %
13+ (options.report, options.report))
14+ out = open('%s.out' % options.report, 'wb')
15 elif options.output == '-':
16 if sys.version[0] < '3':
17 out = sys.stdout

Subscribers

People subscribed via source and target branches