Skip to content

Commit

Permalink
Added Test Cases and fixed some edge logic in santisation methods.
Browse files Browse the repository at this point in the history
  • Loading branch information
hr2904 committed Jun 11, 2024
1 parent 38e49b6 commit f88b042
Show file tree
Hide file tree
Showing 4 changed files with 172 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
import org.apache.commons.collections.functors.NotPredicate;
import org.apache.commons.lang.StringUtils;

import java.io.UnsupportedEncodingException;
import java.net.URLDecoder;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
Expand Down Expand Up @@ -181,9 +183,13 @@ public static boolean validateFilePath(String fileToImport) {
String allowedDirectory = "/var/app/allowed/";

try {
Path normalizedPath = Paths.get(fileToImport).normalize();
// Decode URL-encoded characters first
String decodedPath = URLDecoder.decode(fileToImport, "UTF-8");

if (fileToImport.contains("..") || fileToImport.contains("./") || fileToImport.contains(".\\")) {
Path normalizedPath = Paths.get(decodedPath).normalize();

// Check for directory traversal attempts after decoding
if (decodedPath.contains("..") || decodedPath.contains("./") || decodedPath.contains(".\\")) {
return false;
}

Expand All @@ -196,7 +202,10 @@ public static boolean validateFilePath(String fileToImport) {
}

return true;
} catch (UnsupportedEncodingException e) {
return false;
} catch (Exception e) {
// Handle other exceptions, such as those thrown by Paths.get() for invalid paths
return false;
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package org.apache.atlas.repository.util;

import org.junit.Test;

import static org.apache.atlas.repository.util.FilterUtil.validateFilePath;
import static org.junit.Assert.*;

public class FilterUtilTest {
@Test
public void testValidateFilePath_ValidPath() {
assertTrue("Should return true for a valid path within the allowed directory.",
validateFilePath("/var/app/allowed/file.txt"));
}

@Test
public void testValidateFilePath_RelativeTraversal() {
assertFalse("Should return false for a path attempting directory traversal.",
validateFilePath("/var/app/allowed/../notallowed/file.txt"));
}

@Test
public void testValidateFilePath_DotSlash() {
assertFalse("Should return false for a path with relative current directory notation.",
validateFilePath("/var/app/allowed/./file.txt"));
}

@Test
public void testValidateFilePath_BackSlash() {
assertFalse("Should return false for a path with mixed slash types potentially bypassing checks.",
validateFilePath("/var/app/allowed/.\\file.txt"));
}

@Test
public void testValidateFilePath_NotAbsolute() {
assertFalse("Should return false for non-absolute paths.",
validateFilePath("var/app/allowed/file.txt"));
}

@Test
public void testValidateFilePath_AbsoluteButOutsideAllowed() {
assertFalse("Should return false for absolute paths that do not start with the allowed directory.",
validateFilePath("/var/app/notallowed/file.txt"));
}

@Test
public void testValidateFilePath_WithUnusualCharacters() {
assertFalse("Should return false for paths with unusual characters aiming to navigate directories.",
validateFilePath("/var/app/allowed/..\\file.txt"));
}

@Test
public void testValidateFilePath_WithEncodedTraversal() {
assertFalse("Should return false for paths with URL-encoded traversal sequences.",
validateFilePath("/var/app/allowed/%2e%2e/notallowed/file.txt"));
}

@Test
public void testValidateFilePath_CatchException() {
assertFalse("Should return false for paths that cause exceptions, like those containing null bytes.",
validateFilePath("/var/app/allowed/\0file.txt"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -244,11 +244,22 @@ private void handleRedirect(HttpServletRequest servletRequest, HttpServletRespon
httpServletResponse.sendRedirect(sanitizedLocation);
}
}
private static String sanitizeRedirectLocation(String redirectLocation) {
public static String sanitizeRedirectLocation(String redirectLocation) {
if (redirectLocation == null) return null;
try {
String encodedUrl = URLEncoder.encode(redirectLocation, "UTF-8");
return encodedUrl.replaceAll("\\r", "").replaceAll("\\n", "");
// Remove CR and LF characters to preemptively prevent response splitting
String preProcessedUrl = redirectLocation.replace("\r", "").replace("\n", "");

// Encode any percent signs not already part of a percent-encoded sequence
preProcessedUrl = preProcessedUrl.replaceAll("%(?![0-9a-fA-F]{2})", "%25");

// URL encode the entire string
String encodedUrl = URLEncoder.encode(preProcessedUrl, "UTF-8");

// Normalize encoded sequences that might be affected by double encoding
encodedUrl = encodedUrl.replaceAll("%25([0-9a-fA-F]{2})", "%$1");

return encodedUrl;
} catch (UnsupportedEncodingException e) {
throw new RuntimeException("UTF-8 encoding not supported", e);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
package org.apache.atlas.web.filters;

import org.junit.Test;


import static org.apache.atlas.web.filters.ActiveServerFilter.sanitizeRedirectLocation;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;

public class MetaStoreActiveServerFilterTest {

@Test
public void testSanitizeRedirectLocation_WithValidUrl() {
String testUrl = "http://example.com/page?param=value";
String expected = "http%3A%2F%2Fexample.com%2Fpage%3Fparam%3Dvalue";
String actual = sanitizeRedirectLocation(testUrl);
assertEquals("The URLs do not match.",expected, actual);
}

@Test
public void testSanitizeRedirectLocation_WithNull() {
assertNull("Output should be null for null input.",sanitizeRedirectLocation(null));
}




@Test
public void testSanitizeRedirectLocation_WithSpecialCharacters() {
String testUrl = "http://example.com/page?param=value&another=one";
String expected = "http%3A%2F%2Fexample.com%2Fpage%3Fparam%3Dvalue%26another%3Done";
String actual = sanitizeRedirectLocation(testUrl);
assertEquals("Special characters should be URL encoded.", expected, actual);
}

@Test
public void testSanitizeRedirectLocation_CorruptingCharactersForHttpSplitting() {
String testUrl = "http://example.com/page?param=value%Set-Cookie: test=evil";
String expected = "http%3A%2F%2Fexample.com%2Fpage%3Fparam%3Dvalue%25Set-Cookie%3A+test%3Devil";
String actual = sanitizeRedirectLocation(testUrl);
assertEquals("HTTP response splitting characters and other specials should be properly encoded.", expected, actual);
}

@Test
public void testSanitizeRedirectLocation_MultiLineQueryParameter() {
String testUrl = "http://example.com/search?query=value\n<script>alert('xss')</script>";
String expected = "http%3A%2F%2Fexample.com%2Fsearch%3Fquery%3Dvalue%3Cscript%3Ealert%28%27xss%27%29%3C%2Fscript%3E";
String actual = sanitizeRedirectLocation(testUrl);
assertEquals("Multi-line and script injection attempts should be encoded.", expected, actual);
}


@Test
public void testSanitizeRedirectLocation_CRLFInjectionToSplitResponse() {
String testUrl = "http://example.com/update?action=edit%HTTP/1.1 200 OKContent-Type: text/html";
String expected = "http%3A%2F%2Fexample.com%2Fupdate%3Faction%3Dedit%25HTTP%2F1.1+200+OKContent-Type%3A+text%2Fhtml";
String actual = sanitizeRedirectLocation(testUrl);
assertEquals("CRLF characters used to split HTTP responses should be properly encoded.", expected, actual);
}

@Test
public void testSanitizeRedirectLocation_HeaderInjectionViaNewline() {
String testUrl = "http://example.com/login?redirect=success%Set-Cookie: sessionId=12345";
String expected = "http%3A%2F%2Fexample.com%2Flogin%3Fredirect%3Dsuccess%25Set-Cookie%3A+sessionId%3D12345";
String actual = sanitizeRedirectLocation(testUrl);
assertEquals("Characters potentially harmful for HTTP response splitting should be encoded.", expected, actual);
}

@Test
public void testSanitizeRedirectLocation_CRLFRemoved() {
String testUrl = "http://example.com/page\r";
String expected = "http%3A%2F%2Fexample.com%2Fpage";
String actual = sanitizeRedirectLocation(testUrl);
assertEquals("Carriage return characters should be removed.", expected, actual);
}

@Test
public void testSanitizeRedirectLocation_EncodedLineBreaks() {
String testUrl = "http://example.com/page?next=url%0D%0AContent-Length: %300";
String expected = "http%3A%2F%2Fexample.com%2Fpage%3Fnext%3Durl%0D%0AContent-Length%3A+%300";
String actual = sanitizeRedirectLocation(testUrl);
assertEquals("Encoded line breaks and attempts to continue headers should be removed.", expected, actual);
}

}

0 comments on commit f88b042

Please sign in to comment.