CVE-2019-18212

@@ -1,5 +1,23 @@
 # Change Log
 
+## [0.9.1](https://github.com/angelozerr/lsp4xml/milestone/11?closed=1) (October 17, 2019)
+
+### Bug Fixes
+
+ * XSD: IntelliSense and element substitutions. See [#568](https://github.com/angelozerr/lsp4xml/pull/568)
+ * Completion doesn't use file cache for included XML schema. See [#570](https://github.com/angelozerr/lsp4xml/pull/570)
+ * Prevent from NPE validation with schemaLocaton and "schema.reference.4" error. See [#569](https://github.com/angelozerr/lsp4xml/pull/569)
+
+### Performance
+
+ * Improve performance and memory for validation by caching XML Schema / DTD. See [#534](https://github.com/angelozerr/lsp4xml/issues/534)
+
+### Others
+
+ * Update lsp4j version to 0.8.1. See [#571](https://github.com/angelozerr/lsp4xml/pull/571)
+ * Reject download of resource which are not in the cache folder. See [#567](https://github.com/angelozerr/lsp4xml/pull/567)
+ * Add disallowDocTypeDecl & resolveExternalEntities validation settings. See [#566](https://github.com/angelozerr/lsp4xml/pull/566)
+
 ## [0.9.0](https://github.com/angelozerr/lsp4xml/milestone/10?closed=1) (September 10, 2019)
 
 ### Enhancements

@@ -161,27 +161,36 @@ public CompletableFuture<Path> publishDiagnostics(DOMDocument xmlDocument,
 			publishDiagnostics.accept(new PublishDiagnosticsParams(uri, diagnostics));

 			return null;

 		} catch (CacheResourceDownloadingException e) {

-			// An XML Schema or DTD is being downloaded by the cache manager, but it takes

-			// too long.

-			// In this case:

-			// - 1) we add an information message to the document element to explain that

-			// validation

-			// cannot be performed because the XML Schema/DTD is downloading.

-			publishOneDiagnosticInRoot(xmlDocument, e.getMessage(), DiagnosticSeverity.Information, publishDiagnostics);

-			// - 2) we restart the validation only once the XML Schema/DTD is downloaded.

-			e.getFuture() //

-					.exceptionally(downloadException -> {

-						// Error while downloading the XML Schema/DTD

-						publishOneDiagnosticInRoot(xmlDocument, downloadException.getCause().getMessage(),

-								DiagnosticSeverity.Error, publishDiagnostics);

-						return null;

-					}) //

-					.thenAccept((path) -> {

-						if (path != null) {

-							triggerValidation.accept(document);

-						}

-					});

-			return e.getFuture();

+			CompletableFuture<Path> future = e.getFuture();

+			if (future == null) {

+				// This case comes from when URL uses ../../ and resources is not included in

+				// the cache path

+				// To prevent from "Path Traversal leading to Remote Command Execution (RCE)"

+				publishOneDiagnosticInRoot(xmlDocument, e.getMessage(), DiagnosticSeverity.Error, publishDiagnostics);

+			} else {

+				// An XML Schema or DTD is being downloaded by the cache manager, but it takes

+				// too long.

+				// In this case:

+				// - 1) we add an information message to the document element to explain that

+				// validation

+				// cannot be performed because the XML Schema/DTD is downloading.

+				publishOneDiagnosticInRoot(xmlDocument, e.getMessage(), DiagnosticSeverity.Information,

+						publishDiagnostics);

+				// - 2) we restart the validation only once the XML Schema/DTD is downloaded.

+				future //

+						.exceptionally(downloadException -> {

+							// Error while downloading the XML Schema/DTD

+							publishOneDiagnosticInRoot(xmlDocument, downloadException.getCause().getMessage(),

+									DiagnosticSeverity.Error, publishDiagnostics);

+							return null;

+						}) //

+						.thenAccept((path) -> {

+							if (path != null) {

+								triggerValidation.accept(document);

+							}

+						});

+			}

+			return future;

 		}

 	}

 

@@ -24,10 +24,18 @@ public class CacheResourceDownloadingException extends RuntimeException {
 

 	private static final String RESOURCE_LOADING_MSG = "The resource ''{0}'' is downloading.";

 

+	private static final String RESOURCE_NOT_IN_DEPLOYED_PATH_MSG = "The resource ''{0}'' cannot be downloaded in the cache path.";

+	

 	private final String resourceURI;

 

 	private final CompletableFuture<Path> future;

-	

+

+	public CacheResourceDownloadingException(String resourceURI) {

+		super(MessageFormat.format(RESOURCE_NOT_IN_DEPLOYED_PATH_MSG, resourceURI));

+		this.resourceURI = resourceURI;

+		this.future = null;

+	}

+

 	public CacheResourceDownloadingException(String resourceURI, CompletableFuture<Path> future) {

 		super(MessageFormat.format(RESOURCE_LOADING_MSG, resourceURI));

 		this.resourceURI = resourceURI;

@@ -31,13 +31,12 @@
 import java.util.logging.Level;

 import java.util.logging.Logger;

 

-import com.google.common.cache.Cache;

-import com.google.common.cache.CacheBuilder;

-

-

 import org.eclipse.lsp4xml.utils.FilesUtils;

 import org.eclipse.lsp4xml.utils.URIUtils;

 

+import com.google.common.cache.Cache;

+import com.google.common.cache.CacheBuilder;

+

 /**

  * Cache resources manager.

  *

@@ -98,8 +97,10 @@ public Path getResource(final String resourceURI) throws IOException {
 		if (Files.exists(resourceCachePath)) {

 			return resourceCachePath;

 		}

-

-		if(unavailableURICache.getIfPresent(resourceURI) != null) {

+		if (!FilesUtils.isIncludedInDeployedPath(resourceCachePath)) {

+			throw new CacheResourceDownloadingException(resourceURI);

+		}

+		if (unavailableURICache.getIfPresent(resourceURI) != null) {

 			LOGGER.info("Ignored unavailable schema URI: " + resourceURI + "\n");

 			return null;

 		}

@@ -161,7 +162,8 @@ private CompletableFuture<Path> downloadResource(final String resourceURI, Path
 				String error = "[" + rootCause.getClass().getTypeName() + "] " + rootCause.getMessage();

 				LOGGER.log(Level.SEVERE,

 						"Error while downloading " + resourceURI + " to " + resourceCachePath + " : " + error);

-				throw new CacheResourceDownloadedException("Error while downloading '" + resourceURI + "' to " + resourceCachePath + ".", e);

+				throw new CacheResourceDownloadedException(

+						"Error while downloading '" + resourceURI + "' to " + resourceCachePath + ".", e);

 			} finally {

 				synchronized (resourcesLoading) {

 					resourcesLoading.remove(resourceURI);

@@ -20,6 +20,9 @@
 import java.nio.file.Files;

 import java.nio.file.Path;

 import java.nio.file.Paths;

+import java.nio.file.attribute.PosixFilePermissions;

+import java.util.logging.Level;

+import java.util.logging.Logger;

 import java.util.regex.Matcher;

 import java.util.regex.Pattern;

 

@@ -32,6 +35,8 @@
  */

 public class FilesUtils {

 

+	private static final Logger LOGGER = Logger.getLogger(FilesUtils.class.getName());

+

 	public static final String FILE_SCHEME = "file://";

 	public static final String LSP4XML_WORKDIR_KEY = "lsp4xml.workdir";

 	private static String cachePathSetting = null;

@@ -143,6 +148,15 @@ public static void saveToFile(String content, Path outFile) throws IOException {
 		try (Writer writer = Files.newBufferedWriter(outFile, StandardCharsets.UTF_8)) {

 			writer.write(content);

 		}

+		// Make sure it's not executable

+		try {

+			Files.setPosixFilePermissions(outFile, PosixFilePermissions.fromString("rw-r-----"));

+		} catch (UnsupportedOperationException e) {

+			// The associated file system does not support the PosixFileAttributeView,

+			// ignore the error

+			LOGGER.log(Level.SEVERE,

+					"The associated file system '" + outFile + "' + does not support the PosixFileAttributeView", e);

+		}

 	}

 

 	public static int getOffsetAfterScheme(String uri) {

@@ -162,7 +176,8 @@ public static int getOffsetAfterScheme(String uri) {
 	 * the path to that, else it will try to find the parent directory of the

 	 * givenPath.

 	 * 

-	 * **IMPORTANT** The slashes of the given paths have to match the supported OS file path slash

+	 * **IMPORTANT** The slashes of the given paths have to match the supported OS

+	 * file path slash

 	 * 

 	 * @param parentDirectory The directory that the given path is relative to

 	 * @param givenPath       Path that could be absolute or relative

@@ -178,13 +193,11 @@ public static Path getNormalizedPath(String parentDirectory, String givenPath) {
 

 		// in case the given path is incomplete, trim the end

 		String givenPathCleaned;

-		if(lastIndexOfSlash == 0) { // Looks like `/someFileOrFolder`

+		if (lastIndexOfSlash == 0) { // Looks like `/someFileOrFolder`

 			return Paths.get(SLASH);

-		}

-		else {

+		} else {

 			givenPathCleaned = lastIndexOfSlash > -1 ? givenPath.substring(0, lastIndexOfSlash) : null;

 		}

-		

 

 		Path p;

 

@@ -201,16 +214,14 @@ public static Path getNormalizedPath(String parentDirectory, String givenPath) {
 			return p;

 		}

 

-

-

 		if (parentDirectory == null) {

 			return null;

 		}

 

 		if (parentDirectory.endsWith(SLASH)) {

 			parentDirectory = parentDirectory.substring(0, parentDirectory.length() - 1);

 		}

-		

+

 		String combinedPath = parentDirectory + SLASH + givenPath;

 		p = getPathIfExists(combinedPath);

 		if (p != null) {

@@ -237,8 +248,9 @@ private static Path getPathIfExists(String path) {
 	}

 

 	/**

-	 * Returns the slash ("/" or "\") that is used by the given string.

-	 * If no slash is given "/" is returned by default.

+	 * Returns the slash ("/" or "\") that is used by the given string. If no slash

+	 * is given "/" is returned by default.

+	 * 

 	 * @param text

 	 * @return

 	 */

@@ -250,18 +262,18 @@ public static String getFilePathSlash(String text) {
 	}

 

 	/**

-	 * Ensures there is no slash before a drive letter, and

-	 * forces use of '\'

+	 * Ensures there is no slash before a drive letter, and forces use of '\'

+	 * 

 	 * @param pathString

 	 * @return

 	 */

 	public static String convertToWindowsPath(String pathString) {

 		String pathSlash = getFilePathSlash(pathString);

-		if(pathString.startsWith(pathSlash) ) {

-			if(pathString.length() > 3) {

+		if (pathString.startsWith(pathSlash)) {

+			if (pathString.length() > 3) {

 				char letter = pathString.charAt(1);

 				char colon = pathString.charAt(2);

-				if(Character.isLetter(letter) && ':' == colon) {

+				if (Character.isLetter(letter) && ':' == colon) {

 					pathString = pathString.substring(1);

 				}

 			}

@@ -273,4 +285,8 @@ public static boolean pathEndsWithFile(String pathString) {
 		Matcher m = endFilePattern.matcher(pathString);

 		return m.matches();

 	}

+

+	public static boolean isIncludedInDeployedPath(Path resourceCachePath) {

+		return resourceCachePath.normalize().startsWith(DEPLOYED_BASE_PATH.get());

+	}

 }

@@ -16,23 +16,24 @@
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.fail;
 
+import java.io.IOException;
 import java.util.concurrent.TimeUnit;
 
-import com.google.common.cache.Cache;
-import com.google.common.cache.CacheBuilder;
-
 import org.eclipse.lsp4xml.AbstractCacheBasedTest;
 import org.junit.After;
+import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 
+import com.google.common.cache.Cache;
+import com.google.common.cache.CacheBuilder;
+
 public class CacheResourcesManagerTest extends AbstractCacheBasedTest {
 
 	private CacheResourcesManager cacheResourcesManager;
 
 	private FileServer server;
 
-	
 	@Before
 	public void setup() throws Exception {
 		cacheResourcesManager = new CacheResourcesManager(testingCache());
@@ -71,12 +72,12 @@ public void testUnavailableCache() throws Exception {
 		} catch (CacheResourceDownloadingException ignored) {
 		}
 		TimeUnit.MILLISECONDS.sleep(200);
-		//failed to download so returns null
+		// failed to download so returns null
 		assertNull(cacheResourcesManager.getResource(uri));
 
-		TimeUnit.SECONDS.sleep(1);//wait past the cache expiration date
+		TimeUnit.SECONDS.sleep(1);// wait past the cache expiration date
 
-		//Manager should retry downloading
+		// Manager should retry downloading
 		try {
 			cacheResourcesManager.getResource(uri);
 			fail("cacheResourcesManager should be busy re-downloading the url");
@@ -98,12 +99,26 @@ public void testAvailableCache() throws Exception {
 		assertNotNull(cacheResourcesManager.getResource(uri));
 
 		server.stop();
-		TimeUnit.SECONDS.sleep(1);//wait past the cache expiration date
+		TimeUnit.SECONDS.sleep(1);// wait past the cache expiration date
 
-		//Manager should return cached content, even if server is offline
+		// Manager should return cached content, even if server is offline
 		assertNotNull(cacheResourcesManager.getResource(uri));
 	}
 
+	@Test
+	public void testGetBadResource() throws IOException {
+		CacheResourceDownloadingException actual = null;
+		try {
+			cacheResourcesManager.getResource("http://localhost/../../../../../test.txt");
+		} catch (CacheResourceDownloadingException e) {
+			actual = e;
+		}
+		Assert.assertNotNull(actual);
+		Assert.assertEquals(
+				"The resource 'http://localhost/../../../../../test.txt' cannot be downloaded in the cache path.",
+				actual.getMessage());
+	}
+
 	private Cache<String, Boolean> testingCache() {
 		return CacheBuilder.newBuilder().expireAfterWrite(1, TimeUnit.SECONDS).maximumSize(1).build();
 	}