Skip to content

Commit

Permalink
Arbitrary file access during archive extraction ("Zip Slip")
Browse files Browse the repository at this point in the history
  • Loading branch information
kevin-mcgoldrick committed Jul 24, 2024
1 parent 55aea12 commit b53a6d1
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ public class StandaloneAgentStartup implements Runnable {
private static String API_HARNESS_COMMAND = "./startAgent.sh";
public static final String METHOD_SETTINGS = "/settings";
public static final String METHOD_SUPPORT = "/support-files";
private static final String TANK_AGENT_DIR = "/opt/tank_agent";
private static final long PING_TIME = 1000 * 60 * 5;// five minutes

private String controllerBase;
Expand Down Expand Up @@ -74,24 +75,25 @@ public void startTest(final StandaloneAgentRequest request) {
try {
currentAvailability.setAvailabilityStatus(AgentAvailabilityStatus.DELEGATING);
sendAvailability();
LOG.info("Starting up: ControllerBaseUrl=" + controllerBase);
LOG.info("Starting up: ControllerBaseUrl={}", controllerBase);
URL url = new URL(controllerBase + SERVICE_RELATIVE_PATH + METHOD_SETTINGS);
LOG.info("Starting up: making call to tank service url to get settings.xml "
+ url.toExternalForm());
LOG.info("Starting up: making call to tank service url to get settings.xml {}", url.toExternalForm());
try ( InputStream settingsStream = url.openStream() ) {
String settings = IOUtils.toString(settingsStream, StandardCharsets.UTF_8);
FileUtils.writeStringToFile(new File("settings.xml"), settings, StandardCharsets.UTF_8);
FileUtils.writeStringToFile(new File(TANK_AGENT_DIR, "settings.xml"), settings, StandardCharsets.UTF_8);
LOG.info("got settings file...");
}
url = new URL(controllerBase + SERVICE_RELATIVE_PATH + METHOD_SUPPORT);
LOG.info("Making call to tank service url to get support files " + url.toExternalForm());
LOG.info("Making call to tank service url to get support files {}", url.toExternalForm());
try( ZipInputStream zip = new ZipInputStream(url.openStream()) ) {
ZipEntry entry = zip.getNextEntry();
while (entry != null) {
String name = entry.getName();
LOG.info("Got file from controller: " + name);
File f = new File(name);
try ( FileOutputStream fout = FileUtils.openOutputStream(f) ){
LOG.info("Got file from controller: {}", name);
File file = new File(TANK_AGENT_DIR, name);
if (!file.toPath().normalize().startsWith(TANK_AGENT_DIR)) // Protect "Zip Slip"
throw new Exception("Bad zip entry");
try ( FileOutputStream fout = FileUtils.openOutputStream(file) ){
IOUtils.copy(zip, fout);
}
entry = zip.getNextEntry();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ public class AgentStartup implements Runnable {
private static final String METHOD_SETTINGS = "/settings";
private static final String API_HARNESS_COMMAND = "./startAgent.sh";
private static final String METHOD_SUPPORT = "/support-files";
private static final String TANK_AGENT_DIR = "/opt/tank_agent";
private static final int[] FIBONACCI = new int[] { 1, 1, 2, 3, 5, 8, 13 };

private final String controllerBaseUrl;
Expand All @@ -53,20 +54,20 @@ public void run() {
logger.info("Starting up...");
HttpClient client = HttpClient.newHttpClient();
try {
logger.info("Starting up: ControllerBaseUrl=" + controllerBaseUrl);
logger.info("Starting up: ControllerBaseUrl={}", controllerBaseUrl);
HttpRequest request = HttpRequest.newBuilder().uri(URI.create(
controllerBaseUrl + SERVICE_RELATIVE_PATH + METHOD_SETTINGS))
.header("Authorization", "bearer "+token).build();
logger.info("Starting up: making call to tank service url to get settings.xml " +
controllerBaseUrl + SERVICE_RELATIVE_PATH + METHOD_SUPPORT);
client.send(request, BodyHandlers.ofFile(Paths.get("settings.xml")));
logger.info("Starting up: making call to tank service url to get settings.xml {} {} {}",
controllerBaseUrl, SERVICE_RELATIVE_PATH, METHOD_SUPPORT);
client.send(request, BodyHandlers.ofFile(Paths.get(TANK_AGENT_DIR, "settings.xml")));
logger.info("got settings file...");
// Download Support Files
request = HttpRequest.newBuilder().uri(URI.create(
controllerBaseUrl + SERVICE_RELATIVE_PATH + METHOD_SUPPORT))
.header("Authorization", "bearer "+token).build();
logger.info("Making call to tank service url to get support files " +
controllerBaseUrl + SERVICE_RELATIVE_PATH + METHOD_SUPPORT);
logger.info("Making call to tank service url to get support files {} {} {}",
controllerBaseUrl, SERVICE_RELATIVE_PATH, METHOD_SUPPORT);
int retryCount = 0;
while (true) {
try (ZipInputStream zip = new ZipInputStream(
Expand All @@ -75,33 +76,37 @@ public void run() {
while (entry != null) {
String name = entry.getName();
logger.info("Got file from controller: " + name);
File f = new File(name);
try (FileOutputStream fout = FileUtils.openOutputStream(f)) {
File file = new File(TANK_AGENT_DIR, name);
if (!file.toPath().normalize().startsWith(TANK_AGENT_DIR)) // Protect "Zip Slip"
throw new Exception("Bad zip entry");
try (FileOutputStream fout = FileUtils.openOutputStream(file)) {
IOUtils.copy(zip, fout);
}
entry = zip.getNextEntry();
}
break;
} catch (EOFException | ZipException e) {
logger.error("Error unzipping support files : retryCount="
+ retryCount + " : " + e.getMessage());
logger.error("Error unzipping support files : retryCount={} : {}", retryCount, e.getMessage());
if (retryCount < FIBONACCI.length) {
Thread.sleep( FIBONACCI[retryCount++] * 1000 );
} else throw e;
}
}
// now start the harness
String jvmArgs = AmazonUtil.getUserDataAsMap().get(TankConstants.KEY_JVM_ARGS);
logger.info("Starting apiharness with command: "
+ API_HARNESS_COMMAND + " -http=" + controllerBaseUrl + " " + jvmArgs);
logger.info("Starting apiharness with command: {} \" -http=\" {} {}",
API_HARNESS_COMMAND, controllerBaseUrl, jvmArgs);
Runtime.getRuntime().exec(
new String[] {API_HARNESS_COMMAND, "-http=", controllerBaseUrl, jvmArgs});
} catch (ConnectException ce) {
logger.error("Error creating connection to "
+ controllerBaseUrl + " : this is normal during the bake : " + ce.getMessage());
logger.error("Error creating connection to {} : this is normal during the bake : {}",
controllerBaseUrl, ce.getMessage());
} catch (IOException e) {
logger.error("Error Executing API Harness Command: " + API_HARNESS_COMMAND + ": " + e, e);
} catch (InterruptedException ignored) {}
logger.error("Error Executing API Harness Command: {} : {}", API_HARNESS_COMMAND, e, e);
} catch (InterruptedException ignored) {
} catch (Exception e) {
logger.error("Error in AgentStartup {}", e, e);
}
}

public static void main(String[] args) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,23 @@ public FileSystemFileStorage(String basePath, boolean compress) {

@Override
public void storeFileData(FileData fileData, InputStream input) {
File file = new File(FilenameUtils.normalize(basePath + "/" + fileData.getPath() + "/" + fileData.getFileName()));
if (!file.getParentFile().exists()) {
file.getParentFile().mkdirs();
}
try (OutputStream output = compress ?
new GZIPOutputStream(new FileOutputStream(file)) :
new FileOutputStream(file) ) {
IOUtils.copy(input, output);
} catch (IOException e) {
LOG.error("Error storing file: " + e, e);
try {
File file = new File(FilenameUtils.normalize(basePath + "/" + fileData.getPath() + "/" + fileData.getFileName()));
if (!file.toPath().normalize().startsWith(basePath)) // Protect "Zip Slip"
throw new Exception("Bad zip entry");
if (!file.getParentFile().exists()) {
file.getParentFile().mkdirs();
}
try (OutputStream output = compress ?
new GZIPOutputStream(new FileOutputStream(file)) :
new FileOutputStream(file)) {
IOUtils.copy(input, output);
} catch (IOException e) {
LOG.error("Error storing file: {}", e, e);
throw new RuntimeException(e);
}
} catch (Exception e) {
LOG.error("Error storing file: {}", e, e);
throw new RuntimeException(e);
}
}
Expand Down

0 comments on commit b53a6d1

Please sign in to comment.