Skip to content

Commit

Permalink
Merge pull request #473 from rvinjamu/master
Browse files Browse the repository at this point in the history 
Fix for Security Issue : https://bugs.eclipse.org/bugs/show_bug.cgi?id=538142
  • Loading branch information
Yulin Wang committed Sep 28, 2018
2 parents 91eae5e + f119e6e commit e63581a
Show file tree
Hide file tree
Showing 4 changed files with 114 additions and 8 deletions.
106 changes: 99 additions & 7 deletions ...port.viewer/birt/WEB-INF/classes/org/eclipse/birt/report/context/ViewerAttributeBean.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,15 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;

import javax.servlet.http.HttpServletRequest;

import org.apache.axis.AxisFault;
import org.eclipse.birt.report.IBirtConstants;
import org.eclipse.birt.report.engine.api.EngineException;
import org.eclipse.birt.report.engine.api.IReportDocument;
Expand Down Expand Up @@ -69,6 +72,15 @@
public class ViewerAttributeBean extends BaseAttributeBean
{

/* white list of extensions for rptdocument to be produced */
private static Set<String> allowedExtensionsForRptDocument;

/* black list of extensions for the rptdocument to be produced*/
private static Set<String> disallowedExtensionsForRptDocument;

private static final String KEY_RPTDOC_ALLOWED_EXTENSIONS = "reportdocument.allowed-extensions";
private static final String KEY_RPTDOC_DISALLOWED_EXTENSIONS = "reportdocument.disallowed-extensions";

/**
* Report parameters as string map
*/
Expand Down Expand Up @@ -106,6 +118,37 @@ public class ViewerAttributeBean extends BaseAttributeBean

private Boolean reportRtl;



static
{
allowedExtensionsForRptDocument = new HashSet<String>();
disallowedExtensionsForRptDocument = new HashSet<String>();

String allowedExtString = (String) ParameterAccessor.getInitProp(KEY_RPTDOC_ALLOWED_EXTENSIONS);
if (allowedExtString != null && allowedExtString.trim().length() > 0)
{
String[] allowedExtArray = allowedExtString.trim().split(",");
for(String s: allowedExtArray)
{
allowedExtensionsForRptDocument.add(s.trim());
}
}

String disallowedExtString = (String) ParameterAccessor.getInitProp(KEY_RPTDOC_DISALLOWED_EXTENSIONS);
if (disallowedExtString != null && disallowedExtString.trim().length() > 0)
{
String[] disallowedExtArray = disallowedExtString.trim().split(",");
for(String s: disallowedExtArray)
{
disallowedExtensionsForRptDocument.add(s.trim());
}
}

}



/**
* Constructor.
*
Expand All @@ -132,10 +175,11 @@ public ViewerAttributeBean( HttpServletRequest request )
protected void __init( HttpServletRequest request ) throws Exception
{
// If GetImage operate, return directly.
String servletPath = request.getServletPath( );
if ( ParameterAccessor.isGetImageOperator( request )
&& ( IBirtConstants.SERVLET_PATH_FRAMESET.equalsIgnoreCase( request.getServletPath( ) )
|| IBirtConstants.SERVLET_PATH_OUTPUT.equalsIgnoreCase( request.getServletPath( ) )
|| IBirtConstants.SERVLET_PATH_RUN.equalsIgnoreCase( request.getServletPath( ) ) || IBirtConstants.SERVLET_PATH_PREVIEW.equalsIgnoreCase( request.getServletPath( ) ) ) )
&& ( IBirtConstants.SERVLET_PATH_FRAMESET.equalsIgnoreCase( servletPath )
|| IBirtConstants.SERVLET_PATH_OUTPUT.equalsIgnoreCase( servletPath )
|| IBirtConstants.SERVLET_PATH_RUN.equalsIgnoreCase( servletPath ) || IBirtConstants.SERVLET_PATH_PREVIEW.equalsIgnoreCase( servletPath ) ) )
{
return;
}
Expand All @@ -158,13 +202,22 @@ protected void __init( HttpServletRequest request ) throws Exception
this.reportPageRange = ParameterAccessor.getPageRange( request );
this.action = ParameterAccessor.getAction( request );

boolean checkReportDocumentExtension = false;

// If use frameset/output/download/extract servlet pattern, generate
// document
// from design file
if ( IBirtConstants.SERVLET_PATH_FRAMESET.equalsIgnoreCase( request.getServletPath( ) )
|| IBirtConstants.SERVLET_PATH_OUTPUT.equalsIgnoreCase( request.getServletPath( ) )
|| IBirtConstants.SERVLET_PATH_DOWNLOAD.equalsIgnoreCase( request.getServletPath( ) )
|| IBirtConstants.SERVLET_PATH_EXTRACT.equalsIgnoreCase( request.getServletPath( ) ) )
if ( IBirtConstants.SERVLET_PATH_FRAMESET.equalsIgnoreCase( servletPath )
|| IBirtConstants.SERVLET_PATH_OUTPUT.equalsIgnoreCase( servletPath )
)
{
this.reportDocumentName = ParameterAccessor.getReportDocument( request,
null,
true );
checkReportDocumentExtension = true;
}
else if (IBirtConstants.SERVLET_PATH_DOWNLOAD.equalsIgnoreCase( servletPath )
|| IBirtConstants.SERVLET_PATH_EXTRACT.equalsIgnoreCase( servletPath ))
{
this.reportDocumentName = ParameterAccessor.getReportDocument( request,
null,
Expand All @@ -175,6 +228,15 @@ protected void __init( HttpServletRequest request ) throws Exception
this.reportDocumentName = ParameterAccessor.getReportDocument( request,
null,
false );
if (IBirtConstants.SERVLET_PATH_DOCUMENT.equalsIgnoreCase( servletPath ) && reportDocumentName != null)
{
checkReportDocumentExtension = true;
}
}
// Fix for security issue: https://bugs.eclipse.org/bugs/show_bug.cgi?id=538142
if (checkReportDocumentExtension)
{
checkExtensionAllowedForRPTDocument(this.reportDocumentName);
}

this.reportDesignName = ParameterAccessor.getReport( request, null );
Expand Down Expand Up @@ -1251,4 +1313,34 @@ public boolean isReportRtl( )

return ( reportRtl != null ) ? reportRtl.booleanValue( ) : false;
}

protected static void checkExtensionAllowedForRPTDocument(String rptDocumentName) throws ViewerException
{
int extIndex = rptDocumentName.lastIndexOf(".");
String extension = null;
boolean validExtension = true;

if (extIndex > -1 && (extIndex+1) < rptDocumentName.length())
{
extension = rptDocumentName.substring(extIndex + 1);

if ( !disallowedExtensionsForRptDocument.isEmpty() &&
disallowedExtensionsForRptDocument.contains(extension))
{
validExtension = false;
}

if ( !allowedExtensionsForRptDocument.isEmpty() && !allowedExtensionsForRptDocument.contains( extension ))
{
validExtension = false;
}

if (!validExtension)
{
throw new ViewerException(BirtResources
.getMessage( ResourceConstants.ERROR_INVALID_EXTENSION_FOR_DOCUMENT_PARAMETER, new String[] {extension} ) );
}

}
}
}
2 changes: 1 addition & 1 deletion ...t.report.viewer/birt/WEB-INF/classes/org/eclipse/birt/report/resource/Messages.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ birt.viewer.error.viewingsessionexpired=The viewing session has expired.
birt.viewer.error.viewingsessionlocked=The viewing session is locked and can't be terminated.
birt.viewer.error.viewingsessionmaxreached=The maximum number of viewing sessions has been reached.
birt.viewer.error.columnrequired=No column is selected, please select at least one column for export.

birt.viewer.error.invalidextfordocumentparam=Invalid extension - "{0}" for the __document parameter.
birt.viewer.message.taskcanceled=Current operation has been canceled.

###############################################################################
Expand Down
1 change: 1 addition & 0 deletions ...eport.viewer/birt/WEB-INF/classes/org/eclipse/birt/report/resource/ResourceConstants.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ public interface ResourceConstants
public static final String GENERAL_ERROR_VIEWING_SESSION_EXPIRED = "birt.viewer.error.viewingsessionexpired"; //$NON-NLS-1$
public static final String GENERAL_ERROR_VIEWING_SESSION_LOCKED = "birt.viewer.error.viewingsessionlocked"; //$NON-NLS-1$
public static final String GENERAL_ERROR_VIEWING_SESSION_MAX_REACHED = "birt.viewer.error.viewingsessionmaxreached"; //$NON-NLS-1$
public static final String ERROR_INVALID_EXTENSION_FOR_DOCUMENT_PARAMETER = "birt.viewer.error.invalidextfordocumentparam"; //$NON-NLS-1$

// general exception
public static final String GENERAL_EXCEPTION_DOCUMENT_FILE_ERROR = "birt.viewer.generalException.DOCUMENT_FILE_ERROR"; //$NON-NLS-1$
Expand Down
13 changes: 13 additions & 0 deletions viewer/org.eclipse.birt.report.viewer/birt/WEB-INF/viewer.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,22 @@ viewer.session.maximumSessionCount=0
# cleant by this mechanism.
viewer.session.maximumSessionCountPolicy=1


#Restrictions on the __document parameter when used to specify the report document to be generated. These restrictions
#are only applicable for actions like frameset, document, output which generate a report document. Please note that irrespective
#of the settting here, when the __document param is expected and not specified, the system uses rptdocument as the extension for the
#target report document. To maintain consistency, do not specify rptdocument in the black list and if white list is defined, add rptdocument to the list.

#Comma separated white list of extensions for the report document produced by the system.
reportdocument.allowed-extensions=
#Comma separated black list of extensions for the report document produced by the system. The black list takes precedence over the white list.
reportdocument.disallowed-extensions=jsp


# [LOGGERS]
# "logger."+class=level
# if no level is specified or the text "DEFAULT",
# then the default level from the web.xml will be used
logger.org.eclipse.datatools.connectivity.oda=DEFAULT
logger.org.eclipse.datatools.enablement.oda=DEFAULT

0 comments on commit e63581a

Please sign in to comment.