Fix vulnerability in imported filename allocation. (#4237)

Follow up for https://github.com/OpenRefine/OpenRefine/pull/3048.
Code change suggested by https://github.com/Marcono1234.

Closes #3043.
This commit is contained in:
Antonin Delpeuch 2021-10-23 09:01:14 +02:00 committed by GitHub
parent 226a1baeea
commit 30a0f6643d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 23 additions and 7 deletions

View File

@ -45,6 +45,7 @@ import java.io.UnsupportedEncodingException;
import java.net.URL;
import java.net.URLConnection;
import java.nio.charset.Charset;
import java.nio.file.Path;
import java.text.NumberFormat;
import java.util.ArrayList;
import java.util.Collections;
@ -53,6 +54,7 @@ import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Properties;
import java.util.stream.Collectors;
import java.util.zip.GZIPInputStream;
import java.util.zip.ZipEntry;
import java.util.zip.ZipInputStream;
@ -75,7 +77,6 @@ import org.apache.hc.core5.http.ContentType;
import org.apache.hc.core5.http.HttpEntity;
import org.apache.hc.core5.http.HttpStatus;
import org.apache.hc.core5.http.io.HttpClientResponseHandler;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@ -89,7 +90,6 @@ import com.google.refine.model.Project;
import com.google.refine.util.HttpClient;
import com.google.refine.util.JSONUtilities;
import com.google.refine.util.ParsingUtilities;
import java.util.stream.Collectors;
public class ImportingUtilities {
final static protected Logger logger = LoggerFactory.getLogger("importing-utilities");
@ -444,17 +444,20 @@ public class ImportingUtilities {
}
File file = new File(dir, name);
Path normalizedFile = file.toPath().normalize();
// For CVE-2018-19859, issue #1840
if (!file.toPath().normalize().startsWith(dir.toPath().normalize() + File.separator)) {
if (!normalizedFile.startsWith(dir.toPath().normalize() + File.separator)) {
throw new IllegalArgumentException("Zip archives with files escaping their root directory are not allowed.");
}
int dot = name.lastIndexOf('.');
String prefix = dot < 0 ? name : name.substring(0, dot);
String suffix = dot < 0 ? "" : name.substring(dot);
Path normalizedParent = normalizedFile.getParent();
String fileName = normalizedFile.getFileName().toString();
int dot = fileName.lastIndexOf('.');
String prefix = dot < 0 ? fileName : fileName.substring(0, dot);
String suffix = dot < 0 ? "" : fileName.substring(dot);
int index = 2;
while (file.exists()) {
file = new File(dir, prefix + "-" + index++ + suffix);
file = normalizedParent.resolve(prefix + "-" + index++ + suffix).toFile();
}
file.getParentFile().mkdirs();

View File

@ -96,6 +96,19 @@ public class ImportingUtilitiesTests extends ImporterTest {
ImportingUtilities.allocateFile(tempDir, "../../tmp/script.sh");
}
@Test
public void testAllocateFileDeduplication() throws IOException {
// Test for comment https://github.com/OpenRefine/OpenRefine/issues/3043#issuecomment-671057317
File tempDir = TestUtils.createTempDirectory("openrefine-allocate-file-test");
File dirA = new File(tempDir, "a");
dirA.mkdir();
File conflicting = new File(dirA, "dummy");
conflicting.createNewFile();
File allocated = ImportingUtilities.allocateFile(dirA, ".././a/dummy");
Assert.assertEquals(allocated, new File(dirA, "dummy-2"));
}
@Test
public void urlImporting() throws IOException {