Opened 10 years ago

Closed 10 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)

sagenb-7495.patch (4.0 KB) - added by was 10 years ago.
sagenb-7495.2.patch (5.3 KB) - added by mpatel 10 years ago.
Version 2. Minor simplifications. Apply only this patch to sagenb repo.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 10 years ago by was

  • Priority changed from critical to blocker

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.

http://localhost:8000/home/admin/13/do_upload_data?urlField=%27%27&nameField=../../../../../../../../tmp/xyz

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.

comment:2 Changed 10 years ago by jason

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 10 years ago by was

  • 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 10 years ago by was

comment:4 follow-up: Changed 10 years ago by mpatel

Creating a new file with name & raises an OSError.

comment:5 in reply to: ↑ 4 Changed 10 years ago by mpatel

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/'

Changed 10 years ago by mpatel

Version 2. Minor simplifications. Apply only this patch to sagenb repo.

comment:6 follow-up: Changed 10 years ago by mpatel

I think the OSError above is a separate URI encoding problem.

comment:7 Changed 10 years ago by mpatel

  • Authors set to William Stein
  • 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 10 years ago by mpatel

Replying to mpatel:

I think the OSError above is a separate URI encoding problem.

This is now #7500.

comment:9 Changed 10 years ago by was

  • 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 10 years ago by was

  • Resolution set to fixed
  • Status changed from positive_review to closed

merged into sage-4.3

Note: See TracTickets for help on using tickets.