Skip to content

Commit

Permalink
fix: avoid arbitrary file access during archive extraction ("Zip Slip")
Browse files Browse the repository at this point in the history 
Ensure that output paths constructed from zip archive entries are validated to prevent writing files to unexpected locations.

ING-4369
  • Loading branch information
@emanuelaepure10
emanuelaepure10 committed Jul 12, 2024
1 parent 23bcc04 commit 5f2bbf2
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 112 deletions.
62 changes: 34 additions & 28 deletions ...ommon.core/src/eu/esdihumboldt/hale/common/core/io/project/impl/ArchiveProjectImport.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,42 +87,48 @@ public URI getProjectLocation() {
// destination file has to be a directory
private void getZipFiles(InputStream file, File destination) throws IOException {
byte[] buf = new byte[1024];
ZipInputStream zipinputstream = null;
ZipEntry zipentry;
zipinputstream = new ZipInputStream(file);

try {
while ((zipentry = zipinputstream.getNextEntry()) != null) {
String entryName = zipentry.getName();
int n;
FileOutputStream fileoutputstream;
File newFile = new File(entryName);
String directory = newFile.getParent();

if (directory != null) {
File newF = new File(destination, directory);
try {
newF.mkdirs();
} catch (SecurityException e) {
log.debug("Can not create directories because of SecurityManager", e);
}
}
try (ZipInputStream zipInputStream = new ZipInputStream(file)) {
ZipEntry zipEntry;

File nf = new File(destination, entryName);
fileoutputstream = new FileOutputStream(nf);
while ((zipEntry = zipInputStream.getNextEntry()) != null) {
String entryName = zipEntry.getName();

while ((n = zipinputstream.read(buf, 0, 1024)) > -1)
fileoutputstream.write(buf, 0, n);
// Normalize the entry name and ensure it doesn't point outside
// the destination directory
File newFile = new File(destination, entryName).getCanonicalFile();

fileoutputstream.close();
zipinputstream.closeEntry();
if (!newFile.getPath()
.startsWith(destination.getCanonicalPath() + File.separator)) {
throw new IOException(
"Entry is outside of the target dir: " + zipEntry.getName());
}

if (zipEntry.isDirectory()) {
if (!newFile.isDirectory() && !newFile.mkdirs()) {
throw new IOException("Failed to create directory " + newFile);
}
}
else {
// Ensure parent directories are created
File parent = newFile.getParentFile();
if (!parent.isDirectory() && !parent.mkdirs()) {
throw new IOException("Failed to create directory " + parent);
}

// Write the file content
try (FileOutputStream fos = new FileOutputStream(newFile)) {
int len;
while ((len = zipInputStream.read(buf)) > 0) {
fos.write(buf, 0, len);
}
}
}

zipInputStream.closeEntry();
}
} catch (FileNotFoundException e) {
throw new IOException("Destination directory not found", e);
}

zipinputstream.close();
}

@Override
Expand Down
124 changes: 59 additions & 65 deletions platform/tools/Servicemerger/src/Servicemerger.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -281,72 +281,66 @@ private void writeMergedFile(File file, String text) throws IOException{
* @param dir the directory of the jar files
* @throws IOException
*/
private void merge(File dir) throws IOException{

File[] files = dir.listFiles();
Map <String, File> visited = new HashMap<String, File>();
//are jar files in the given directory?
if (files != null) {
for (int i = 0; i < files.length; i++) {

//we only work with .jar files
JarFile jar = null;

if(this.bndFlag == false){
if(files[i].getName().endsWith(".jar")){
jar = new JarFile(files[i]);
}
}

else if(this.bndFlag == true){
if(files[i].getName().endsWith(".jar") && bndSpecifications.contains(files[i].getName())){
jar = new JarFile(files[i]);
}
}

if(jar != null){

Enumeration<JarEntry> jarenu = jar.entries();
//lets look at the entry of a specific jar file
while (jarenu.hasMoreElements()){

JarEntry entry = jarenu.nextElement();
//do the jar file contains a servicefile?
if(entry.getName().contains("META-INF")
&& entry.getName().contains("services")
&& !entry.isDirectory()){

if(visited.containsKey(entry.getName())){

File mergedFile = visited.get(entry.getName());
writeMergedFile(mergedFile, readFile(jar.getInputStream(entry)));

}

else {
File mergedFile = new File(dest + java.io.File.separator + entry.getName());
writeMergedFile(mergedFile, readFile(jar.getInputStream(entry)));
visited.put(entry.getName(), mergedFile);
}

}



}

}





}
}
private void merge(File dir) throws IOException {
File[] files = dir.listFiles();
Map<String, File> visited = new HashMap<>();

// Are jar files in the given directory?
if (files != null) {
for (File file : files) {
if (file.getName().endsWith(".jar") && shouldProcessJar(file)) {
try (JarFile jar = new JarFile(file)) {
processJarFile(jar, visited);
}
}
}
}
}

private boolean shouldProcessJar(File file) {
return !this.bndFlag || bndSpecifications.contains(file.getName());
}

private void processJarFile(JarFile jar, Map<String, File> visited) throws IOException {
Enumeration<JarEntry> jarEntries = jar.entries();
while (jarEntries.hasMoreElements()) {
JarEntry entry = jarEntries.nextElement();
if (isServiceFile(entry)) {
File mergedFile = visited.get(entry.getName());
if (mergedFile == null) {
mergedFile = new File(dest, entry.getName());
if (isInsideDestinationDirectory(mergedFile)) {
visited.put(entry.getName(), mergedFile);
} else {
throw new IOException("Entry is outside of the target dir: " + entry.getName());
}
}
writeMergedFile(mergedFile, readJarFile(jar.getInputStream(entry)));
}
}
}

private boolean isServiceFile(JarEntry entry) {
return entry.getName().contains("META-INF") && entry.getName().contains("services") && !entry.isDirectory();
}

private boolean isInsideDestinationDirectory(File file) throws IOException {
Path destDirPath = Paths.get(dest).toAbsolutePath();
Path filePath = file.toPath().toAbsolutePath();
return filePath.startsWith(destDirPath);
}

private byte[] readJarFile(InputStream inputStream) throws IOException {
return inputStream.readAllBytes();
}

private void writeMergedFile(File file, byte[] content) throws IOException {
// Append content to the file, creating directories if necessary
if (!file.getParentFile().exists()) {
file.getParentFile().mkdirs();
}
Files.write(file.toPath(), content, java.nio.file.StandardOpenOption.CREATE, java.nio.file.StandardOpenOption.APPEND);
}




/**
* main method to start the Servicemerger
Expand Down
48 changes: 29 additions & 19 deletions util/plugins/eu.esdihumboldt.util/src/eu/esdihumboldt/util/reflection/ReflectionHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,9 @@ public static Method findSetter(Class<?> c, String property, Class<?> propertyTy
Method result = null;
Method[] methods = c.getMethods();
for (Method m : methods) {
if (m.getName().equals(setterName)
&& m.getParameterTypes().length == 1
&& (propertyType == null || m.getParameterTypes()[0]
.isAssignableFrom(propertyType))) {
if (m.getName().equals(setterName) && m.getParameterTypes().length == 1
&& (propertyType == null
|| m.getParameterTypes()[0].isAssignableFrom(propertyType))) {
result = m;
break;
}
Expand Down Expand Up @@ -109,10 +108,9 @@ public static Method findDeepSetter(Class<?> c, String property, Class<?> proper
// search visible and hidden methods in this class
Method[] methods = c.getDeclaredMethods();
for (Method m : methods) {
if (m.getName().equals(setterName)
&& m.getParameterTypes().length == 1
&& (propertyType == null || m.getParameterTypes()[0]
.isAssignableFrom(propertyType))) {
if (m.getName().equals(setterName) && m.getParameterTypes().length == 1
&& (propertyType == null
|| m.getParameterTypes()[0].isAssignableFrom(propertyType))) {
result = m;
break;
}
Expand Down Expand Up @@ -355,8 +353,8 @@ public static void setDeepProperty(Object bean, String propertyName, Object valu
* @throws InvocationTargetException if the getter throws an exception
*/
@SuppressWarnings("unchecked")
private static <T> T invokeGetter(Object bean, Method getter) throws IllegalArgumentException,
IllegalAccessException, InvocationTargetException {
private static <T> T invokeGetter(Object bean, Method getter)
throws IllegalArgumentException, IllegalAccessException, InvocationTargetException {
boolean accessible = getter.isAccessible();
getter.setAccessible(true);
T result = null;
Expand Down Expand Up @@ -432,8 +430,8 @@ public static <T> T getDeepProperty(Object bean, String propertyName)
* @throws IllegalStateException is the field could be found or accessed
*/
@SuppressWarnings("unchecked")
public static <T> T getDeepPropertyOrField(Object bean, String propertyName, Class<T> valueClass)
throws IllegalArgumentException, InvocationTargetException {
public static <T> T getDeepPropertyOrField(Object bean, String propertyName,
Class<T> valueClass) throws IllegalArgumentException, InvocationTargetException {
try {
return ReflectionHelper.getDeepProperty(bean, propertyName);
} catch (NoSuchMethodException e) {/* ignore */
Expand All @@ -443,8 +441,8 @@ public static <T> T getDeepPropertyOrField(Object bean, String propertyName, Cla
// there is no getter for the property. try to get the field directly
Field f = findDeepField(bean.getClass(), propertyName, valueClass);
if (f == null) {
throw new IllegalStateException("Could not find " + "field for property "
+ propertyName + " in class " + bean.getClass().getCanonicalName());
throw new IllegalStateException("Could not find " + "field for property " + propertyName
+ " in class " + bean.getClass().getCanonicalName());
}

boolean access = f.isAccessible();
Expand Down Expand Up @@ -562,12 +560,24 @@ public static synchronized File[] getFilesFromPackage(String pkg) throws IOExcep
}
while (entries.hasMoreElements()) {
JarEntry j = entries.nextElement();
if (j.getName().matches("^" + package_path + ".+\\..+")) { //$NON-NLS-1$ //$NON-NLS-2$
String entryName = j.getName();

// Ensure the entry name is properly normalized and checked
File targetFile = new File(package_path, entryName);
String canonicalTargetPath = targetFile.getCanonicalPath();
String canonicalBasePath = new File(package_path).getCanonicalPath();

if (!canonicalTargetPath.startsWith(canonicalBasePath + File.separator)) {
throw new IOException(
"Entry is outside of the target directory: " + entryName);
}

if (entryName.matches("^" + package_path + ".+\\..+")) { //$NON-NLS-1$ //$NON-NLS-2$
if (slashed) {
file_names.add("/" + j.getName()); //$NON-NLS-1$
file_names.add("/" + entryName); //$NON-NLS-1$
}
else {
file_names.add(j.getName());
file_names.add(entryName);
}
}
}
Expand Down Expand Up @@ -664,8 +674,8 @@ private static void getSubPackagesFromPackage(String pkg, List<String> l, boolea
}
}

private static void getClassesFromPackage(String pkg, List<Class<?>> l,
ClassLoader classLoader, boolean recursive) throws IOException {
private static void getClassesFromPackage(String pkg, List<Class<?>> l, ClassLoader classLoader,
boolean recursive) throws IOException {
File[] files = getFilesFromPackage(pkg);
for (File f : files) {
String name = f.getName();
Expand Down

0 comments on commit 5f2bbf2

Please sign in to comment.