Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cross-site reference vulnerability #319

Open
kcrisman opened this issue Dec 19, 2014 · 10 comments

Comments

@kcrisman
Copy link
Contributor

commented Dec 19, 2014

Not a lot of public notebooks out there nowadays. But http://trac.sagemath.org/ticket/8840 points out something nasty that could be done, most actions in fact.

@migeruhito

This comment has been minimized.

Copy link
Contributor

commented Dec 20, 2014

I'm very interested in this issue. Please, might anybody give me a working example? The ​five years old example in http://trac.sagemath.org/ticket/8840 doesn't seems available.

@kcrisman

This comment has been minimized.

Copy link
Contributor Author

commented Dec 21, 2014

I don't know enough php to do exactly what that person suggests, but I think that if one knew how to write a php redirect along those lines, you would just have to make it redirect to something like your.sage.notebook.com/upload?url=foo.png or something like that. Sorry I can't be of more help on that.

@migeruhito

This comment has been minimized.

Copy link
Contributor

commented Dec 21, 2014

Sorry for my previous question. After reading #252 and #100, I see now the XSS problem and the subsequent potential CSRF vulnerability. Thanks.

@migeruhito

This comment has been minimized.

Copy link
Contributor

commented Dec 21, 2014

Now, I found several forms to exploit the XSS vulnerability. This affect the text and output cells, as said in #100.

Also, I think the worksheet interface is broken. Example:

  • Create a new worksheet
  • Enter in the first cell
///
}}}
{{{
  • Save & quit (don't evaluate).
  • Edit the worksheet. An unexpected new cell appears.

I don't know if this is the designed behavior and for this reason I've not opened a new issue. In any case it is not the reasonable behavior for a mere Notebook user.
Due to this, the XSS can also be triggered from an input cell with the normal worksheet interface (not the text edit mode) from an ordinary input cell.

@migeruhito

This comment has been minimized.

Copy link
Contributor

commented Dec 21, 2014

Disabling the publishing system is not sufficient. Any user can attack any other via the sharing mechanism.

@kcrisman

This comment has been minimized.

Copy link
Contributor Author

commented Dec 22, 2014

Any user can attack any other via the sharing mechanism.

I suppose that is true, but that is a much more subtle issue, because in principle you should only be opening shared worksheets from someone you know (like your teacher). And it isn't an outside vulnerability, so one could ban that user but not have to worry about your host taking the whole server down because of this.

Also, I think the worksheet interface is broken.

That is really intriguing. I don't think it is expected behavior but then again I don't think anyone would be surprised at this behavior, because you have to know the internal structure to "randomly" type this in. If there is an easy way to disable this (such as some check that the number of input cells is the same when there haven't been any created with some click or evaluate action, outside of Edit mode) that would be nice, but I don't know if it is.

@migeruhito

This comment has been minimized.

Copy link
Contributor

commented Dec 23, 2014

Also, I think the worksheet interface is broken.

Assumptions about what an user can or cannot type must not be made by a GUI designer. A GUI must be monkey proof. This is a necessary condition for an user friendly GUI.

That said, I agree with you that an input as of the previous example is a highly unlike one. But this is not my point. Substitute the input of previous example by the legal one:

my_fancy_dict = { 
1: {2:
     {3: 4
}}}

and see. This is only an example, but creative people might surely do all kind of funny things. An easy workaround is not feasible. I think that I might write more than one solution. The general recipe is all characters or group of them with an special meaning in a coding system must be escaped when they act as themselves, but then, the actual problem would be backward compatibility.

@migeruhito

This comment has been minimized.

Copy link
Contributor

commented Dec 23, 2014

With respect to the XSS problem:

I suppose that is true, but that is a much more subtle issue...

Injection of (malicious) code via sharing, publishing or uploading (or even copy/paste in Edit mode, or in a input cell by exploiting the bug previously illustrated) is a fact in the current Notebook implementation. And in some situations this can be important. Suppose that someone (like me) set up a classroom server and the student have important worksheets for their ratings hosted there. A malicious student could share an infected worksheet and destroy the work of his friend. It might then unshare and delete the evil worksheet. Or better it might use a compromised account of a third student for his despicable activities. OK, the 99% of my students don't want to do (and can't do) that. But this 1% (or 0,001%) makes security questions matter. This is only an example. And yes, I know that most of the people runs Sage in a single user environment, but also in this case it is dangerous to download this fancy Sage worksheet that I've seen in that cool site. This is an outside vulnerability.

I'm looking for a solution. But the changeset (to actually get rid of the problem in a radical way) will be enormous as far as I know. At this moment I'm thinking in a radical redesign of the cell displaying mechanisms, and I will try it for my next year classroom server. I hope that in the meantime somebody gives a shorter and more effective solution.

@kcrisman

This comment has been minimized.

Copy link
Contributor Author

commented Dec 24, 2014

OK, the 99% of my students don't want to do (and can't do) that. But this 1% (or 0,001%) makes security questions matter.

Of course. I do wish that people had thought about this from the beginning, but of course I am only capable of maintaining this, not of coming up with the notebook in the first place! Any ideas you have I will seriously look at.

@gutow

This comment has been minimized.

Copy link
Contributor

commented Dec 30, 2014

I believe my fix for proxying (#328) may solve some of this as almost all links in the notebook (except the docs) are now specified with absolute paths. The only way to take over the site or redirect somebody somewhere else is to do deep rewriting of every link on each page (possible, but requires a lot of processing). That said, there are definitely issues of accessing each other's worksheets that are built into the design of the notebook.

I think the best solution is to use a robust CMS system as the front end. They have already solved the security problems. Then the issue is just having the interactive interface available when editing a worksheet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.