Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: arbitrary file access during archive extraction ("Zip Slip") #1215

Merged
merged 1 commit into from
Jul 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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());
Dismissed Show dismissed Hide dismissed
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
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
Loading