Opened 12 years ago
Closed 12 years ago
#7495 closed defect (fixed)
notebook: fix massive security vulnerability and get rid of all possible "internal server errors" when doing "Data --> Upload or attach file"
Reported by: | was | Owned by: | boothby |
---|---|---|---|
Priority: | blocker | Milestone: | sage-4.3 |
Component: | notebook | Keywords: | |
Cc: | Merged in: | ||
Authors: | William Stein | Reviewers: | Mitesh Patel |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
Uploading or attaching an empty file or a file that doesn't exist, etc. can cause internal server errors instead of a proper error message.
Moreover, notice these lines in twist.py:
class Worksheet_do_upload_data ... dest = os.path.join(self.worksheet.data_directory(), name) if os.path.exists(dest): os.unlink(dest)
With a properly crafted URL I bet name could contain .. and hence one could make the notebook *server* delete any file it has access to! This is a critical security vulnerability.
See also #3849 for similar issues when uploading a worksheet.
Attachments (2)
Change History (12)
comment:1 Changed 12 years ago by
- Priority changed from critical to blocker
comment:2 Changed 12 years ago by
Can you do a quick grep through the source to see if any os.* functions are used in a like manner in the notebook?
comment:3 Changed 12 years ago by
- Status changed from new to needs_review
- Summary changed from notebook: get rid of all possible "internal server errors" when doing "Data --> Upload or attach file" to notebook: fix massive security vulnerability and get rid of all possible "internal server errors" when doing "Data --> Upload or attach file"
Changed 12 years ago by
comment:4 follow-up: ↓ 5 Changed 12 years ago by
Creating a new file with name &
raises an OSError.
comment:5 in reply to: ↑ 4 Changed 12 years ago by
Replying to mpatel:
Creating a new file with name
&
raises an OSError.
Actually, clicking to delete this file raises the error:
exceptions.OSError: [Errno 21] Is a directory: '/home/apps/.sage/sage_notebook.sagenb/home/admin/88/data/'
comment:6 follow-up: ↓ 8 Changed 12 years ago by
I think the OSError
above is a separate URI encoding problem.
comment:7 Changed 12 years ago by
- Reviewers set to Mitesh Patel
Anyway, my digression aside, my review is positive, as far as it goes.
comment:8 in reply to: ↑ 6 Changed 12 years ago by
comment:9 Changed 12 years ago by
- Report Upstream set to N/A
- Status changed from needs_review to positive_review
"Anyway, my digression aside, my review is positive, as far as it goes. " so positive review.
comment:10 Changed 12 years ago by
- Resolution set to fixed
- Status changed from positive_review to closed
merged into sage-4.3
Yes, this is fully exploitable and allows one to delete any file on the server: E.g., on my laptop I created a file tmp/xyz, then pasted in this url, and that file was deleted.
With a little more work, one could not only delete any file, but I think *replace* it by a file of ones choice. That's a pretty massive exploit.
So I'm upping this to a blocker and fixing it now.