Skip to content

Directory Traversal: Attacker-controlled Data Used in File Path via request in CustomerController.saveSettings #61

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.

Sign up for GitHub

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

Open
ongamse wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Directory Traversal: Attacker-controlled Data Used in File Path via request in CustomerController.saveSettings #61

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
97 changes: 67 additions & 30 deletions src/main/java/io/shiftleft/controller/CustomerController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,37 +20,74 @@ public void saveSettings(HttpServletResponse httpResponse, WebRequest request) t
String base64txt = cookie[0].replace("settings=","");

// Check md5sum
String cookieMD5sum = cookie[1];
String calcMD5Sum = DigestUtils.md5Hex(base64txt);
if(!cookieMD5sum.equals(calcMD5Sum))
{
httpResponse.getOutputStream().println("Wrong md5");
throw new Exception("Invalid MD5");
}
@RequestMapping(value = "/saveSettings", method = RequestMethod.GET)
public void saveSettings(HttpServletResponse httpResponse, WebRequest request) throws Exception {
if (!checkCookie(request)) {
httpResponse.getOutputStream().println("Error");
throw new Exception("cookie is incorrect");
}

// Now we can store on filesystem
String[] settings = new String(Base64.getDecoder().decode(base64txt)).split(",");
// storage will have ClassPathResource as basepath
ClassPathResource cpr = new ClassPathResource("./static/");
String settingsCookie = request.getHeader("Cookie");
String[] cookie = settingsCookie.split(",");
if (cookie.length < 2) {
httpResponse.getOutputStream().println("Malformed cookie");
throw new Exception("cookie is incorrect");
}

// Whitelist the filename to prevent directory traversal
String filename = settings[0];
if(!filename.matches("[a-zA-Z0-9._-]+")) {
httpResponse.getOutputStream().println("Invalid filename");
throw new Exception("Filename contains invalid characters");
}
String base64txt = cookie[0].replace("settings=", "");
String cookieMD5sum = cookie[1];
String calcMD5Sum = DigestUtils.md5Hex(base64txt);

File file = new File(cpr.getPath()+filename);
if(!file.exists()) {
file.getParentFile().mkdirs();
}
if (!cookieMD5sum.equals(calcMD5Sum)) {
httpResponse.getOutputStream().println("Wrong md5");
throw new Exception("Invalid MD5");
}

FileOutputStream fos = new FileOutputStream(file, true);
// First entry is the filename -> remove it
String[] settingsArr = Arrays.copyOfRange(settings, 1, settings.length);
// on setting at a linez
fos.write(String.join("\n",settingsArr).getBytes());
fos.write(("\n"+cookie[cookie.length-1]).getBytes());
fos.close();
httpResponse.getOutputStream().println("Settings Saved");
}
String[] settings = new String(Base64.getDecoder().decode(base64txt)).split(",");
ClassPathResource cpr = new ClassPathResource("./static/");

// SECURITY FIX: Instead of using user-provided filename, generate a secure filename
// Option 1: Generate UUID-based filename
String userIdentifier = getUserIdentifier(request);
String secureFilename = UUID.randomUUID().toString() + ".settings";

// Option 2: Content-addressed storage approach
String[] settingsArr = Arrays.copyOfRange(settings, 1, settings.length);
String settingsContent = String.join("\n", settingsArr);
String contentHash = DigestUtils.sha256Hex(settingsContent);
String contentAddressedFilename = contentHash + ".settings";

// Use the content-addressed filename for stronger security
Path storageRoot = Paths.get(cpr.getPath()).normalize();
Path secureStoragePath = storageRoot.resolve(contentAddressedFilename).normalize();
File file = secureStoragePath.toFile();

if (!file.exists()) {
file.getParentFile().mkdirs();
}

// Write the settings content to the secure file location
FileOutputStream fos = new FileOutputStream(file, true);
fos.write(settingsContent.getBytes());
fos.write(("\n" + cookie[cookie.length - 1]).getBytes());
fos.close();

// Store mapping for future reference (in production, this would save to database)
storeUserSettingsMapping(userIdentifier, contentAddressedFilename);

httpResponse.getOutputStream().println("Settings Saved");
}

// Helper method to get user identifier from the request
private String getUserIdentifier(WebRequest request) {
// In a real implementation, this would extract authenticated user information
// For example: return userService.getCurrentUserId(request);
return request.getRemoteUser() != null ? request.getRemoteUser() : "anonymous";
}

// Helper method to store user-to-filename mapping
private void storeUserSettingsMapping(String userIdentifier, String filename) {
// In a real implementation, this would store the mapping in a database
// For example: userSettingsRepository.saveMapping(userIdentifier, filename);
System.out.println("Stored mapping: " + userIdentifier + " -> " + filename);
}
Loading