Skip to content

Commit 091819b

Browse files
tdonohuejr-rk
authored andcommitted
Merge pull request DSpace#11036 from DSpace/backport-11031-to-dspace-7_x
[Port dspace-7_x] Improve SAF manifest path handling
1 parent 19e3658 commit 091819b

2 files changed

Lines changed: 40 additions & 1 deletion

File tree

dspace-api/src/main/java/org/dspace/app/itemimport/ItemImportServiceImpl.java

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1037,6 +1037,34 @@ protected void addDCValue(Context c, Item i, String schema, Node n)
10371037
}
10381038
}
10391039

1040+
/**
1041+
* Ensures a file path does not attempt to access files outside the designated parent directory.
1042+
*
1043+
* @param parentDir The absolute path to the parent directory that should contain the file
1044+
* @param fileName The name or path of the file to validate
1045+
* @throws IOException If an error occurs while resolving canonical paths, or the file path attempts
1046+
* to access a location outside the parent directory
1047+
*/
1048+
private void validateFilePath(String parentDir, String fileName) throws IOException {
1049+
File parent = new File(parentDir);
1050+
File file = new File(fileName);
1051+
1052+
// If the fileName is not an absolute path, we resolve it against the parentDir
1053+
if (!file.isAbsolute()) {
1054+
file = new File(parent, fileName);
1055+
}
1056+
1057+
String parentCanonicalPath = parent.getCanonicalPath();
1058+
String fileCanonicalPath = file.getCanonicalPath();
1059+
1060+
if (!fileCanonicalPath.startsWith(parentCanonicalPath)) {
1061+
log.error("File path outside of canonical root requested: fileCanonicalPath={} does not begin " +
1062+
"with parentCanonicalPath={}", fileCanonicalPath, parentCanonicalPath);
1063+
throw new IOException("Illegal file path '" + fileName + "' encountered. This references a path " +
1064+
"outside of the import package. Please see the system logs for more details.");
1065+
}
1066+
}
1067+
10401068
/**
10411069
* Read the collections file inside the item directory. If there
10421070
* is one and it is not empty return a list of collections in
@@ -1237,6 +1265,7 @@ protected List<String> processContentsFile(Context c, Item i, String path,
12371265
sDescription = sDescription.replaceFirst("description:", "");
12381266
}
12391267

1268+
validateFilePath(path, sFilePath);
12401269
registerBitstream(c, i, iAssetstore, sFilePath, sBundle, sDescription);
12411270
logInfo("\tRegistering Bitstream: " + sFilePath
12421271
+ "\tAssetstore: " + iAssetstore
@@ -1450,6 +1479,7 @@ protected void processContentFileEntry(Context c, Item i, String path,
14501479
return;
14511480
}
14521481

1482+
validateFilePath(path, fileName);
14531483
String fullpath = path + File.separatorChar + fileName;
14541484

14551485
// get an input stream

dspace-api/src/main/java/org/dspace/storage/bitstore/DSBitStoreService.java

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import java.io.FileOutputStream;
1313
import java.io.IOException;
1414
import java.io.InputStream;
15+
import java.nio.file.Path;
1516
import java.security.DigestInputStream;
1617
import java.security.MessageDigest;
1718
import java.security.NoSuchAlgorithmException;
@@ -248,7 +249,15 @@ protected File getFile(Bitstream bitstream) throws IOException {
248249
log.debug("Local filename for " + sInternalId + " is "
249250
+ bufFilename.toString());
250251
}
251-
return new File(bufFilename.toString());
252+
File bitstreamFile = new File(bufFilename.toString());
253+
Path normalizedPath = bitstreamFile.toPath().normalize();
254+
if (!normalizedPath.startsWith(baseDir.getAbsolutePath())) {
255+
log.error("Bitstream path outside of assetstore root requested:" +
256+
"bitstream={}, path={}, assetstore={}",
257+
bitstream.getID(), normalizedPath, baseDir.getAbsolutePath());
258+
throw new IOException("Illegal bitstream path constructed");
259+
}
260+
return bitstreamFile;
252261
}
253262

254263
public boolean isRegisteredBitstream(String internalId) {

0 commit comments

Comments
 (0)