408122 Local file read vulnerability in BIRT report viewer image

handler check if the imageID conains './' or '.\'
ilatypov · Sep 9, 2013 · 9fe6e9b · 9fe6e9b
1 parent bd4fba2
commit 9fe6e9b
Showing 1 changed file with 13 additions and 29 deletions.
diff --git a/...pse.birt.report.engine/src/org/eclipse/birt/report/engine/api/HTMLServerImageHandler.java b/...pse.birt.report.engine/src/org/eclipse/birt/report/engine/api/HTMLServerImageHandler.java
@@ -98,20 +98,6 @@ public String onCustomImage( IImage image, Object context )
 		return handleImage( image, context, "custom", false ); //$NON-NLS-1$
 	}
 
-	/**
-	 * returns a unique file name based on a directory and name prefix
-	 * 
-	 * @param imageDir
-	 *            directory to store the image
-	 * @param prefix
-	 *            prefix for the file name
-	 * @return a file name
-	 */
-	protected String createUniqueFileName( String imageDir, String prefix )
-	{
-		return createUniqueFileName( imageDir, prefix, null );
-	}
-
 	/**
 	 * creates a unique tempoary file to store an image
 	 * 
@@ -134,7 +120,7 @@ protected String createUniqueFileName( String imageDir, String prefix,
 			uniCount = genUniqueCount( );
 			file = new File( imageDir + "/" + prefix + uniCount + postfix ); //$NON-NLS-1$
 		} while ( file.exists( ) );
-		
+
 		return prefix + uniCount + postfix;
 	}
 
@@ -203,18 +189,9 @@ protected String handleImage( IImage image, Object context, String prefix,
 				return null;
 			}
 
-			String fileName;
-			File file;
 			String extension = image.getExtension( );
-			if ( extension != null && extension.length( ) > 0 )
-			{
-				fileName = createUniqueFileName( imageDir, prefix, extension ); //$NON-NLS-1$
-			}
-			else
-			{
-				fileName = createUniqueFileName( imageDir, prefix );
-			}
-			file = new File( imageDir, fileName ); //$NON-NLS-1$
+			String fileName = createUniqueFileName( imageDir, prefix, extension );
+			File file = new File(imageDir, fileName); //$NON-NLS-1$
 			try
 			{
 				image.writeImage( file );
@@ -329,16 +306,23 @@ protected String getImageMapID( IImage image )
 	public void getImage( OutputStream out, String imageDir, String imageID )
 			throws EngineException
 	{
-		File image = new File( imageDir, imageID );
-		if ( !image.exists( ) )
+		//As imageID is created by handleImage(), we need check the imageID first to avoid
+		//user uses this API read arbitrary file in disk.
+		if ( imageID.indexOf( "./" ) != -1 || imageID.indexOf( ".\\" ) != -1 )
+		{
+			throw new EngineException(
+					MessageConstants.MISSING_IMAGE_FILE_ERROR ); //$NON-NLS-1$ //$NON-NLS-2$
+		}
+		File imageFile = new File( imageDir, imageID );
+		if ( !imageFile.exists( ) )
 		{
 			throw new EngineException(
 					MessageConstants.MISSING_IMAGE_FILE_ERROR ); //$NON-NLS-1$ //$NON-NLS-2$
 		}
 		InputStream in = null;
 		try
 		{
-			in = new FileInputStream( image );
+			in = new FileInputStream( imageFile );
 			byte[] buffer = new byte[1024];
 			int size = 0;
 			do