Merge lp://qastaging/~jamesodhunt/ubuntu/vivid/upstart/bug-1425685 into lp://qastaging/ubuntu/vivid/upstart
Proposed by
James Hunt
| Status: | Merged |
|---|---|
| Merged at revision: | 1608 |
| Proposed branch: | lp://qastaging/~jamesodhunt/ubuntu/vivid/upstart/bug-1425685 |
| Merge into: | lp://qastaging/ubuntu/vivid/upstart |
| Diff against target: |
37 lines (+18/-2) 2 files modified
debian/changelog (+8/-0) debian/upstart-bin.upstart.cron.daily (+10/-2) |
| To merge this branch: | bzr merge lp://qastaging/~jamesodhunt/ubuntu/vivid/upstart/bug-1425685 |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Colin Watson (community) | Approve | ||
| Dimitri John Ledkov | Pending | ||
|
Review via email:
|
|||
Description of the change
* debian/
- [SECURITY FIX]: Only consider valid session files to avoid possible
privilege escalation. Thanks to halfdog for reporting (LP: #1425685).
This branch provides a basic fix for the reported issue. However, I think we could potentially do better by:
- ensuring the user (extracted from the UPSTART_SESSION value) exists.
- ensuring the pid (extracted from the UPSTART_SESSION value) is running and is an upstart process.
- running /sbin/initctl as the user in question (assuming that user is validated as above). The risk here is that doing so may cause the cron job to block indefinately.
Input from other Upstart develops and the Security Team welcome.
To post a comment you must log in.
Revision history for this message
| Dimitri John Ledkov (xnox) wrote | # |
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
This seems fine as far as it goes, although I think it's still slightly incomplete and you could make it safer with a one-liner.