Community
Participate
Working Groups
Build Identifier: Helios Access below link, please replace the port with the real one on your Eclipse first: http://localhost:1295/help/index.jsp?topic=http://demo.testfire.net In the result page, right column has been redirect to http://demo.testfire.net Reproducible: Always Steps to Reproduce: 1.Access below link, please replace the port with the real one on your Eclipse first: http://localhost:1295/help/index.jsp?topic=http://demo.testfire.net 2.In the result page, right column has been redirect to http://demo.testfire.net 3. Affected urls http://localhost:8777/help/ http://localhost:8777/help/advanced/content.jsp http://localhost:8777/help/advanced/help.jsp http://localhost:8777/help/index.jsp
Are you using the Eclipse application or are you running an information center or standalone help?
(In reply to comment #1) > Are you using the Eclipse application or are you running an information center > or standalone help? Hi, I can see this problem in standalone Eclipse help and information center, and also in some Eclipse applications.
The phishing bug was fixed for infocenters in Eclipse 3.4, see Bug 233466. The reason for continuing to allow http: urls in the topic parameter in workbench mode was that there may be legitimate reasons to allow the help system to be opened while displaying a page from the in the content frame and it does not seem that the phishing vulnerability could be exploited in the workbench case. Can you check which version of Eclipse you were using when you saw the problem in an infocenter?
I'm considering disallowing use of external urls in the topic parameter for the workbench, however I want to ensure that legitimate uses of external URLs by Eclipse components continue to work. If a cheat sheet contains an item like this: <item href="http://www.eclipse.org" title="Open External Help Topic"> <description> In the cheat sheet view click on the (?) to the right of this step - eclipse.org should be opened. </description> </item> It will allow the help system to be opened with eclipse.org showing in the content frame, I don't know how widely this is used but I we should continue to support it, otherwise we end up breaking existing cheat sheets. Similarly for Intro <link label="Link to eclipse.org" url="http://org.eclipse.ui.intro/showHelpTopic?id=http://www.eclipse.org" id="anchorlink" style-id="content-link"> <text>External Link</text> </link> Will create a link which opens help with eclipse.org showing in the content frame. In general any calls which go through defaultHelpUI.displayHelpResource() should be free to display external topics and if I can modify the help system to allow those while restricting external topics elsewhere we can eliminate any possibility of phishing through the topic parameter in workbench mode.
Created attachment 175342 [details] Patch
Created attachment 175428 [details] Patch for UA test
This looks good.
The fix does the trick but I don't like too much that UrlUtil.isValidTopicURL(String) now depends on what has been opened before. From reading the name and the Javadoc of that method I would expect a static checking of my URL. If we proceed with the current fix, then we should rename that method and update the Javadoc. HelpDisplay.setHrefOpenedFromHelpDisplay(String) should be private (I would actually even delete it). org.eclipse.help.internal.webapp.data.UrlUtil.isValidTopicURL(String) // are are always valid ==> // are always valid
Created attachment 176505 [details] Updated patch This patch makes HelpDisplay.setHrefOpenedFromHelpDisplay(String) private and fixes the comment. Regarding the comment about not liking the way we are saving the last href called from within the help system and comparing that in UrlUtil.isValidTopicURL() I feel the same way. The problem is that we don't want to allow urls typed into a web browser to support topic parameters that represent external web pages (with protocol http: or https: ). We do however allow cheat sheets and intro to reference external pages and this ends up creating a help system request with a topic parameter which is external.
Continuing the discussion from the previous comment. If we consider the patch which saves the href last opened from the help display to be solution A I can see two other possible approaches to resolving this bug. Solution B: If we see a topic parameter with an external href instead of drawing the help frames with the content pane showing the external web page we redirect to the external web page and do not show the frames in the help system. Solution C: Do nothing because this is not a severe vulnerability for the workbench help system. In order to exploit the vulnerability the bad guys would have to somehow get the user to click on a link included in e-mail or on a web site which included the correct port number for the running help instance. Since port numbers are generated randomly it seems unlikely that this would succeed. I'm interested in getting an opinion from Dani, or anyone else following this bug as to which of solutions A, B and C make most sense. My rationale for going with solution A was that it would block the vulnerability with the least amount of visible impact on the user.
I think A is fine if we rename isValidTopicURL (use a longer more descriptive name) and add some Javadoc that mentions the connection to the Help display. Another benefit of A is that ChrisA and I have already tested that code. +1 for A with these changes.
Created attachment 176692 [details] Patch incorporating Dani's suggestions
Created attachment 176695 [details] Patch for UA tests including Dani's suggested name change
Patch containing Dani's suggestions applied to 3.6 maintenance stream, and the identical change is in HEAD also. Fixed.
The patch has also been applied to the 3.5 maintenance stream
The patch has also been applied to the 3.4 maintenance stream
Verified on M20100901-0800
This bug is currently marked as a private bug for security purposes. Since the bug is fixed, should it not be open?
We had this discussion at the Architecture meeting a while ago and concluded that we did not want to open these bugs up at the time, but I think the issue should be revisited so I will raise it again.
At the architectural council meeting last week I raised the issue of removing the security lock from bug reports which have been fixed - the conclusion was that we should keep these locked.
Removing security restriction for bugs that have been fixed in 3.6.2 or earlier.
