Skip to content
Permalink
Browse files Browse the repository at this point in the history
Merge pull request from GHSA-7557-4v29-rqw6
Fix directory traversal
  • Loading branch information
sudheesh001 committed Jul 2, 2020
2 parents 5f48476 + 714697b commit 50dd692
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 3 deletions.
7 changes: 4 additions & 3 deletions src/org/loklak/data/DAO.java
Expand Up @@ -99,6 +99,7 @@
import org.loklak.server.*;
import org.loklak.stream.MQTTPublisher;
import org.loklak.tools.DateParser;
import org.loklak.tools.IO;
import org.loklak.tools.OS;
import org.loklak.tools.storage.*;
import org.slf4j.Logger;
Expand Down Expand Up @@ -340,7 +341,7 @@ public static void init(Map<String, String> configMap, Path dataPath) throws Exc
// elasticsearch will probably take some time until it is started up. We do some other stuff meanwhile..

// create and document the data dump dir
assets = new File(datadir, "assets");
assets = new File(datadir, "assets").getAbsoluteFile();
external_data = new File(datadir, "external");
dictionaries = new File(external_data, "dictionaries");
dictionaries.mkdirs();
Expand Down Expand Up @@ -525,8 +526,8 @@ public static Map<String, String> nodeSettings() {
public static File getAssetFile(String screen_name, String id_str, String file) {
String letter0 = ("" + screen_name.charAt(0)).toLowerCase();
String letter1 = ("" + screen_name.charAt(1)).toLowerCase();
File storage_path = new File(new File(new File(assets, letter0), letter1), screen_name);
return new File(storage_path, id_str + "_" + file); // all assets for one user in one file
Path storage_path = IO.resolvePath(assets.toPath(), letter0, letter1, screen_name);
return IO.resolvePath(storage_path, id_str + "_" + file).toFile(); // all assets for one user in one file
}

public static Collection<File> getTweetOwnDumps(int count) {
Expand Down
64 changes: 64 additions & 0 deletions src/org/loklak/tools/IO.java
Expand Up @@ -7,6 +7,7 @@
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.security.*;
import java.security.spec.InvalidKeySpecException;
import java.security.spec.X509EncodedKeySpec;
Expand Down Expand Up @@ -96,4 +97,67 @@ public synchronized static PublicKey decodePublicKey(@Nonnull String encodedKey,
}
return null;
}

/**
* Resolves an untrusted user-specified path against the API's base directory.
* Paths that try to escape the base directory are rejected.
*
* @param baseDirPath the absolute path of the base directory that all
* user-specified paths should be within
* @param userPath the untrusted path provided by the API user, expected to be
* relative to {@code baseDirPath}
*/
public static Path resolvePath(final Path baseDirPath, final Path userPath) {
if (!baseDirPath.isAbsolute()) {
throw new IllegalArgumentException("Base path must be absolute");
}

if (userPath.isAbsolute()) {
throw new IllegalArgumentException("User path must be relative");
}

// Join the two paths together, then normalize so that any ".." elements
// in the userPath can remove parts of baseDirPath.
// (e.g. "/foo/bar/baz" + "../attack" -> "/foo/bar/attack")
final Path resolvedPath = baseDirPath.resolve(userPath).normalize();

// Make sure the resulting path is still within the required directory.
// (In the example above, "/foo/bar/attack" is not.)
if (!resolvedPath.startsWith(baseDirPath)) {
throw new IllegalArgumentException("User path escapes the base path");
}

return resolvedPath;
}

public static Path resolvePath(final Path baseDirPath, final String userPath) {
return resolvePath(baseDirPath, Paths.get(userPath));
}

/**
* Checks each subsequent path to be strictly within the baseDirPath so that
* no path argument leads to directory traversal attack
*
* E.g. /models/ + req.model + '/' + req.lang + /images/ + req.image
* Should be checked for ('models', req.model, req.lang, 'images', req.image)
* that each subsequent element is within the previous and not breaking out by passing
* req.model => ..
* req.lang => ..
* req.image => ../../private/data.json
*
* Since just checking the last argument isn't enough
*
* @param baseDirPath the absolute path of the base directory that all
* user-specified paths should be within
* @param paths the untrusted paths provided by the API user, expected to be
* relative to {@code baseDirPath}
*/
public static Path resolvePath(final Path baseDirPath, final String... paths) {
Path resolved = baseDirPath;
for (String path: paths) {
resolved = resolvePath(resolved, path);
}

return resolved;
}
}

0 comments on commit 50dd692

Please sign in to comment.