Bug 336767 - Security Issue in BIRT Viewer
Summary: Security Issue in BIRT Viewer
Status: VERIFIED FIXED
Alias: None
Product: z_Archived
Classification: Eclipse Foundation
Component: BIRT (show other bugs)
Version: 2.6.1   Edit
Hardware: PC Windows 7
: P3 normal (vote)
Target Milestone: 2.6.2   Edit
Assignee: Zhiqiang Qian CLA
QA Contact: Liwen Chen CLA
URL:
Whiteboard:
Keywords: security
Depends on:
Blocks:
 
Reported: 2011-02-09 16:39 EST by Jason Weathersby CLA
Modified: 2014-03-19 06:23 EDT (History)
6 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jason Weathersby CLA 2011-02-09 16:39:46 EST
As pointed out by Jeff Beard-Shouse from Security PS, the viewer contains a security bug.  Currently the user of the viewer can enter a URL Parameter for the report name.  This parameter can be a URL for a report.  This could allow a malicious user to specify a report design that contains JavaScript to be executed on the server running the viewer.  To work around this I suggest adding the following web.xml setting:

	<context-param>
		<param-name>BIRT_VIEWER_URL_REPORT</param-name>
		<param-value>domain</param-value>
	</context-param>

Valid values would be true, false, and domain.  If the value is set to false or is set to any other value than true or domain the report would not be executed.  If the value is set to true, no check is made and the server administrator assumes the risk.  If the value is set to domain, the url of the report design is checked and compared to the host currently running the viewer.  If they match the report will execute.  This change would require adding the following to ParameterAccessor.java

public static final String INIT_BIRT_VIEWER_URL_REPORT = "BIRT_VIEWER_URL_REPORT"

and the getReport method in the same class could be changed to the following:

	public static String getReport( HttpServletRequest request, String filePath )
	{
		String check_url_support = getInitProp("INIT_BIRT_VIEWER_URL_REPORT");
		
		if ( filePath == null )
		{
			filePath = DataUtil.trimString( getParameter( request, PARAM_REPORT ) );
		}
		filePath = decodeFilePath( request, filePath );
		//may need to add ftp?
		if ((filePath.split("http://").length > 1)||(filePath.split("https://").length > 1)) { 
			
			String dm = request.getServerName();
			if( check_url_support == null || (check_url_support.compareToIgnoreCase("false") == 0)){
				return("urls not supported or");
			}
			if( check_url_support.compareToIgnoreCase("domain") == 0 || check_url_support.compareToIgnoreCase("true") == 0){
				URL rpturl;
				try {
					rpturl = new URL(filePath);
					if( rpturl.getHost().compareTo(dm)==0 || check_url_support.compareToIgnoreCase("true") == 0){
						return filePath;
					}else{
						return("urls not supported or");
					}
				} catch (MalformedURLException e) {

				}
				
			}
		}
		return getRealPathOnWorkingFolder( filePath, request );
	}
Comment 1 Wenfeng Li CLA 2011-02-09 17:24:51 EST
Please 
1) externalize the error message string.
2) Might also need to check other remote protocols, such as ftp
Comment 2 Zhiqiang Qian CLA 2011-02-09 23:50:56 EST
I've incorporated the change with slight difference. A new context parameter URL_REPORT_PATH_POLICY is introduced. The accepted values are "all", "domain", "none", which hold similar meanings with the suggested ones. Also the check is done to all protocols. The change is also available in 3.7/4.0 stream.
Comment 3 Jason Weathersby CLA 2011-02-10 01:48:31 EST
Will this be in the 2.6.2 build?
Comment 4 Zhiqiang Qian CLA 2011-02-10 02:20:09 EST
(In reply to comment #3)
> Will this be in the 2.6.2 build?

Yes, should in 2.6.2 final.
Comment 5 Wayne Beaton CLA 2011-02-11 14:35:43 EST
I'm interested in how we handle disclosure of this vulnerability and the patch. Ultimately, the "committer-only" flag needs to be cleared and this bug made public. The question is when?

Is there a set of individuals that we can copy on this bug as sort of an "inner circle" of concerned parties who can use this information to fix their own systems before we more broadly disclose the issue and fix? I'm thinking of trusted adopters and users.
Comment 6 Wenfeng Li CLA 2011-02-11 14:41:12 EST
I don't know of any adoptors would need access first.  Shall we disclose this right away?
Comment 7 Wayne Beaton CLA 2011-02-11 14:52:36 EST
(In reply to comment #6)
> I don't know of any adoptors would need access first.  Shall we disclose this
> right away?

Flipping the bit is sort of a quiet disclosure. Is this vulnerability significant enough that there is serious risk to users in the wild? Should we be making a more active disclosure?

The risk is that nefarious evil-doers may learn about the vulnerability through this disclosure and exploit it in unpatched installations.

Ultimately, you know your community best.
Comment 8 Zhiqiang Qian CLA 2011-02-12 05:37:50 EST
Here is the summary for the fix, which can be applied to older version of viewer code:

4 files changed:
--------------------------

/org.eclipse.birt.report.viewer/birt/WEB-INF/web.xml
====================================================
* Add a new context parameter:

<!--
	Settings for how to deal with the url report path. e.g. "http://host/repo/test.rptdesign". 
		
	Following values are supported:
		
	<all> 		- All paths.
	<domain>	- Only the paths with host matches current domain. Note the comparison is literal, "127.0.0.1" and "localhost" are considered as different hosts.
	<none> 		- URL paths are not supported.
		
	Defaults to "domain".
-->
<context-param>
	<param-name>URL_REPORT_PATH_POLICY</param-name>
	<param-value>domain</param-value>
</context-param>
=====================================================

/org.eclipse.birt.report.viewer/birt/WEB-INF/classes/org/eclipse/birt/report/utility/ParameterAccessor.java
=====================================================
* Add new constants and variable:

public static final String INIT_PARAM_URL_REPORT_PATH_POLICY = "URL_REPORT_PATH_POLICY"; //$NON-NLS-1$
public static final String POLICY_ALL = "all"; //$NON-NLS-1$
public static final String POLICY_DOMAIN = "domain"; //$NON-NLS-1$
public static final String POLICY_NONE = "none"; //$NON-NLS-1$
public static String urlReportPathPolicy = POLICY_DOMAIN;

* Modify method initParameters( ), add init code:

urlReportPathPolicy = context.getInitParameter( INIT_PARAM_URL_REPORT_PATH_POLICY );

* Modify method isValidFilePath( ) and add code block like:

public static boolean isValidFilePath( HttpServletRequest request,
			String filePath )
	......
	......
if ( filePath == null )
	return false;

// check and aply url report path policy
if ( !POLICY_ALL.equalsIgnoreCase( urlReportPathPolicy ) )
{
	File f = new File( filePath );

	if ( !f.isAbsolute( ) )
	{
		try
		{
			URL url = new URL( filePath );

			if ( POLICY_DOMAIN.equalsIgnoreCase( urlReportPathPolicy ) )
			{
				String dm = request.getServerName( );

				if ( !dm.equals( url.getHost( ) ) )
				{
					return false;
				}
			}
			else
			{
				return false;
			}
		}
		catch ( MalformedURLException e )
		{
			// ignore
		}
	}
}

if ( isWorkingFolderAccessOnly )
{
	...... 
	......
=====================================================

/org.eclipse.birt.report.viewer/birt/WEB-INF/classes/org/eclipse/birt/report/context/ViewerAttributeBean.java
=====================================================
* Update reference to method isValidFilePath();
=====================================================

/org.eclipse.birt.report.viewer/birt/WEB-INF/classes/org/eclipse/birt/report/taglib/util/BirtTagUtil.java
=====================================================
* Update reference to method isValidFilePath();
=====================================================
Comment 9 Paul Clenahan CLA 2011-02-15 14:54:54 EST
The following Bulletin has been sent to birt-dev@eclipse.org and will be posted to the BIRT Newgroup...

Eclipse BIRT Security Bulletin: Web Viewer Example Security

Background and Summary
 
The Eclipse BIRT project provides a Web Viewer Example that illustrates one mechanism for integrating BIRT content into a web application. The Web Viewer Example is a small web application that shows how to both generate and view BIRT content from within an application server. It is intended only as example code and provides capabilities including parameter handling and page navigation.
 
A specific capability provided is the ability to “run” a BIRT design (.rptdesign) located in an arbitrary location accessible via the file system, HTTP or FTP.
 
We expect that most developers would use the Web Viewer Example as an example and modify it for their particular needs, including addressing any security requirements for their application. However, a member of the BIRT community has identified a potential security vulnerability with the Web Viewer Example and this bulletin is intended to alert the rest of the community to the issue in case it applies to their usage of the sample code.
 
Vulnerability
 
The Web Viewer Example includes the ability for a user to enter  the location of the BIRT design to be executed as a parameter to the Web Viewer Example.  This parameter can be a path for a design on the local file system, or a URL for HTTP or FTP access.  A URL parameter could allow a malicious user to specify a BIRT design in an environment controlled by them and not part of the web application hosting the Web Viewer Example. This BIRT design could contain malicious JavaScript code that is then executed on the server running the Web Viewer Example.
 
This vulnerability is only accessible if the application server allows outbound HTTP or FTP requests initiated from an application running in the server and if the URL parameter has been exposed as part of the application.
 
Recommendation and Resolution
 
There are three recommendations for mitigating this vulnerability:
 
* Ensure the application server hosting the Web Viewer Example and/or the network infrastructure does not permit outbound HTTP or FTP requests initiated from within the Web Viewer Example application.
* Modify the Web Viewer Example source code to disable or restrict the feature that allows URL based access to BIRT designs. The Web Viewer Example is intended as an example and all source code is provided. Suggested changes are referenced in Bugzilla 336767. These provide additional control of the URL based access to BIRT designs and enable the feature to be disabled or restricted to only allow designs to be accessed from a known domain
* Upgrade to BIRT 2.6.2 or later (when available) that incorporates the changes referenced in Bugzilla 336767 to easily disable or restrict URL based access to BIRT designs, with the default being to restrict access to designs located on the current domain only.
 
For details, see Bugzilla 336767.
Comment 10 Liwen Chen CLA 2011-02-16 01:38:21 EST
Verified in 2.6.2.v20110215-1647

For none, Url beginning with http, ftp, file:/// will return exception like "....rptdesign does not exist or contains errors."
For domain, only report name url using the same host as running report url, the report will run and show content.
For all, url referring to reports from different host using http, ftp or file:/// will work.
Comment 11 Vasanthakumar R CLA 2014-03-19 06:23:05 EDT
Dear All,

I am working in web application using JSF 2.0 with Birt Report Integrated.

I have done the steps below,
 In web.xml
        <context-param>
		<param-name>BIRT_VIEWER_URL_REPORT</param-name>
		<param-value>domain</param-value>
	</context-param>

I have hosted my web application,Straight away i can copy and paste the url used for generating birt report in different machine and ip's,Even if i am not logged into my application also.

Ex (Employee Details Report)
 http://hostname:8080/projectname/frameset?__report=EmployeeReport.rptdesign&frmDate=04-03-2014&toDate=13-03-2014&Emp=307.



Can anyone suggest me an idea to stop this issue.Its very urgent !!