Skip to content

Commit

Permalink
Improve application module performance and security bounds (#6647)
Browse files Browse the repository at this point in the history
  • Loading branch information
@manuel-alvarez-alvarez
manuel-alvarez-alvarez authored Feb 16, 2024
1 parent b550bc8 commit 167e8f7
Show file tree
Hide file tree
Showing 5 changed files with 253 additions and 115 deletions.
221 changes: 131 additions & 90 deletions dd-java-agent/agent-iast/src/main/java/com/datadog/iast/sink/ApplicationModuleImpl.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
package com.datadog.iast.sink;

import static com.datadog.iast.util.StringUtils.endsWithIgnoreCase;
import static com.datadog.iast.util.StringUtils.substringTrim;
import static datadog.trace.api.telemetry.LogCollector.SEND_TELEMETRY;

import com.datadog.iast.Dependencies;
import com.datadog.iast.model.Evidence;
import com.datadog.iast.model.Location;
Expand All @@ -11,28 +15,37 @@
import java.io.File;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.FileVisitOption;
import java.nio.file.FileVisitResult;
import java.nio.file.FileVisitor;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.List;
import java.nio.file.attribute.BasicFileAttributes;
import java.util.Collection;
import java.util.EnumSet;
import java.util.HashSet;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class ApplicationModuleImpl extends SinkModuleBase implements ApplicationModule {

private static final String CONTEXT_LOADER_LISTENER_PATTERN =
"org\\.springframework\\.web\\.context\\.ContextLoaderListener";
private static final Logger LOGGER = LoggerFactory.getLogger(ApplicationModule.class);

private static final String CONTEXT_LOADER_LISTENER_VALUE =
"org.springframework.web.context.ContextLoaderListener";
/** Bounds for the file visitor depth when trying to locate insecure JSP folders */
private static final int JSP_MAX_WALK_DEPTH = 32;

private static final String DISPATCHER_SERVLET_PATTERN =
"org\\.springframework\\.web\\.servlet\\.DispatcherServlet";
private static final String CONTEXT_LOADER_LISTENER =
"org.springframework.web.context.ContextLoaderListener";

private static final String DISPATCHER_SERVLET_VALUE =
private static final String DISPATCHER_SERVLET =
"org.springframework.web.servlet.DispatcherServlet";

private static final String DEFAULT_HTML_ESCAPE = "defaultHtmlEscape";
Expand All @@ -57,18 +70,18 @@ public class ApplicationModuleImpl extends SinkModuleBase implements Application

public static final String WEB_XML = "web.xml";

private static final String REGEX =
String.join(
"|",
CONTEXT_LOADER_LISTENER_PATTERN,
DISPATCHER_SERVLET_PATTERN,
DEFAULT_HTML_ESCAPE,
TOMCAT_MANAGER_APPLICATION,
LISTINGS_PATTERN,
SESSION_TIMEOUT_START_TAG,
SECURITY_CONSTRAINT_START_TAG);

private static final Pattern PATTERN = Pattern.compile(REGEX);
private static final Pattern PATTERN =
Pattern.compile(
Stream.of(
CONTEXT_LOADER_LISTENER,
DISPATCHER_SERVLET,
DEFAULT_HTML_ESCAPE,
TOMCAT_MANAGER_APPLICATION,
LISTINGS_PATTERN,
SESSION_TIMEOUT_START_TAG,
SECURITY_CONSTRAINT_START_TAG)
.map(Pattern::quote)
.collect(Collectors.joining("|")));

private static final int NO_LINE = -1;

Expand All @@ -78,16 +91,20 @@ public ApplicationModuleImpl(final Dependencies dependencies) {

@Override
public void onRealPath(final @Nullable String realPath) {

if (realPath == null) {
return;
}

final Path root = Paths.get(realPath);
if (!Files.exists(root)) {
return;
}
final AgentSpan span = AgentTracer.activeSpan();
checkInsecureJSPLayout(root, span);
checkWebXmlVulnerabilities(root, span);
}

checkInsecureJSPLayout(realPath, span);

String webXmlContent = webXmlContent(realPath);
private void checkWebXmlVulnerabilities(@Nonnull Path path, AgentSpan span) {
String webXmlContent = webXmlContent(path);
if (webXmlContent == null) {
return;
}
Expand All @@ -99,8 +116,8 @@ public void onRealPath(final @Nullable String realPath) {
while (matcher.find()) {
String match = matcher.group();
switch (match) {
case DISPATCHER_SERVLET_VALUE:
case CONTEXT_LOADER_LISTENER_VALUE:
case DISPATCHER_SERVLET:
case CONTEXT_LOADER_LISTENER:
isSpring = true;
break;
case DEFAULT_HTML_ESCAPE:
Expand All @@ -124,19 +141,19 @@ public void onRealPath(final @Nullable String realPath) {
}

if (isSpring) {
checkDefaultHtmlEscapeInvalid(webXmlContent, span, defaultHtmlEscapeIndex);
checkDefaultHtmlEscapeInvalid(webXmlContent, defaultHtmlEscapeIndex, span);
}
}

private void checkDefaultHtmlEscapeInvalid(
String webXmlContent, AgentSpan span, int defaultHtmlEscapeIndex) {
@Nonnull String webXmlContent, int defaultHtmlEscapeIndex, AgentSpan span) {
if (defaultHtmlEscapeIndex != -1) {
int start =
webXmlContent.indexOf(PARAM_VALUE_START_TAG, defaultHtmlEscapeIndex)
+ PARAM_VALUE_START_TAG.length();
String value =
webXmlContent.substring(start, webXmlContent.indexOf(PARAM_VALUE_END_TAG, start));
if (!value.trim().toLowerCase().equals("true")) {
substringTrim(webXmlContent, start, webXmlContent.indexOf(PARAM_VALUE_END_TAG, start));
if (!value.equalsIgnoreCase("true")) {
report(
span,
VulnerabilityType.DEFAULT_HTML_ESCAPE_INVALID,
Expand All @@ -161,8 +178,8 @@ private void checkDirectoryListingLeak(
int valueIndex =
webXmlContent.indexOf(PARAM_VALUE_START_TAG, index) + PARAM_VALUE_START_TAG.length();
int valueLast = webXmlContent.indexOf(PARAM_VALUE_END_TAG, valueIndex);
String data = webXmlContent.substring(valueIndex, valueLast);
if (data.trim().toLowerCase().equals("true")) {
String data = substringTrim(webXmlContent, valueIndex, valueLast);
if (data.equalsIgnoreCase("true")) {
report(
span,
VulnerabilityType.DIRECTORY_LISTING_LEAK,
Expand All @@ -174,11 +191,10 @@ private void checkDirectoryListingLeak(
private void checkSessionTimeOut(final String webXmlContent, int index, final AgentSpan span) {
try {
String innerText =
webXmlContent
.substring(
index + SESSION_TIMEOUT_START_TAG.length(),
webXmlContent.indexOf(SESSION_TIMEOUT_END_TAG, index))
.trim();
substringTrim(
webXmlContent,
index + SESSION_TIMEOUT_START_TAG.length(),
webXmlContent.indexOf(SESSION_TIMEOUT_END_TAG, index));
int timeoutValue = Integer.parseInt(innerText);
if (timeoutValue > 30 || timeoutValue == -1) {
report(
Expand All @@ -194,11 +210,10 @@ private void checkSessionTimeOut(final String webXmlContent, int index, final Ag

private void checkVerbTampering(final String webXmlContent, int index, final AgentSpan span) {
String innerText =
webXmlContent
.substring(
index + SECURITY_CONSTRAINT_START_TAG.length(),
webXmlContent.indexOf(SECURITY_CONSTRAINT_END_TAG, index))
.trim();
substringTrim(
webXmlContent,
index + SECURITY_CONSTRAINT_START_TAG.length(),
webXmlContent.indexOf(SECURITY_CONSTRAINT_END_TAG, index));
if (!innerText.contains("<http-method>")) {
report(
span,
Expand All @@ -208,82 +223,108 @@ private void checkVerbTampering(final String webXmlContent, int index, final Age
}
}

private int getLine(String webXmlContent, int index) {
int line = 1;
int limit = Math.min(index, webXmlContent.length());
for (int i = limit; i > 0; i--) {
if (webXmlContent.charAt(i) == '\n') {
line++;
}
}
return line;
}

private void report(AgentSpan span, VulnerabilityType type, String value, int line) {
reporter.report(
span,
new Vulnerability(
type, Location.forSpanAndFileAndLine(span, WEB_XML, line), new Evidence(value)));
}

private void checkInsecureJSPLayout(String realPath, AgentSpan span) {
List<String> jspPaths = findRecursively(new File(realPath));
private void checkInsecureJSPLayout(@Nonnull Path path, AgentSpan span) {
final Collection<Path> jspPaths = findInsecureJspPaths(path);
if (jspPaths.isEmpty()) {
return;
}
String result =
jspPaths.stream()
.map(s -> File.separatorChar + s)
.map(jspFolder -> relativize(path, jspFolder))
.collect(Collectors.joining(System.lineSeparator()));
reporter.report(
span,
new Vulnerability(
VulnerabilityType.INSECURE_JSP_LAYOUT, Location.forSpan(span), new Evidence(result)));
}

private static int getLine(String webXmlContent, int index) {
int line = 1;
int limit = Math.min(index, webXmlContent.length());
for (int i = limit; i > 0; i--) {
if (webXmlContent.charAt(i) == '\n') {
line++;
}
}
return line;
}

@Nullable
private String webXmlContent(final String realPath) {
if (realPath != null) {
Path path = Paths.get(realPath + File.separatorChar + WEB_INF + File.separatorChar + WEB_XML);
if (path.toFile().exists()) {
try {
return new String(Files.readAllBytes(path), StandardCharsets.UTF_8);
} catch (IOException e) {
// Nothing to do
}
private static String webXmlContent(final Path realPath) {
Path path = realPath.resolve(WEB_INF).resolve(WEB_XML);
if (Files.exists(path)) {
try {
return new String(Files.readAllBytes(path), StandardCharsets.UTF_8);
} catch (IOException e) {
LOGGER.debug(SEND_TELEMETRY, "Failed to read {}, encoding issue?", path, e);
}
}
return null;
}

private List<String> findRecursively(final File root) {
List<String> jspPaths = new ArrayList<>();
if (root.listFiles() != null) {
for (File file : root.listFiles()) {
if (file.isFile() && isJsp(file.getName().toLowerCase())) {
jspPaths.add(file.getName());
} else if (file.isDirectory() && !WEB_INF.equals(file.getName())) {
addDirectoryJSPs("", file, jspPaths);
}
}
private static Collection<Path> findInsecureJspPaths(final Path root) {
try {
final InsecureJspFolderVisitor visitor = new InsecureJspFolderVisitor();
// TODO is it OK to ignore symlinks here?
Files.walkFileTree(root, EnumSet.noneOf(FileVisitOption.class), JSP_MAX_WALK_DEPTH, visitor);
return visitor.folders;
} catch (Exception e) {
throw new RuntimeException(e);
}
return jspPaths;
}

private void addDirectoryJSPs(final String path, final File dir, final List<String> jspPaths) {
String currentPath = path + dir.getName() + File.separatorChar;
if (dir.listFiles() != null) {
for (File file : dir.listFiles()) {
if (file.isFile() && isJsp(file.getName().toLowerCase())) {
jspPaths.add(currentPath + file.getName());
} else if (file.isDirectory()) {
addDirectoryJSPs(currentPath, file, jspPaths);
}
}
private static String relativize(final Path root, final Path path) {
final String relative = root.relativize(path).toString();
if (relative.isEmpty()) {
return File.separator;
}
if (relative.charAt(0) == File.separatorChar) {
return relative;
}
return File.separatorChar + relative;
}

private boolean isJsp(final String name) {
return name.endsWith(".jsp") || name.endsWith(".jspx");
private static class InsecureJspFolderVisitor implements FileVisitor<Path> {
private final Set<Path> folders = new HashSet<>();

@Override
public FileVisitResult preVisitDirectory(final Path dir, final BasicFileAttributes attrs)
throws IOException {
final String folder = dir.getFileName().toString();
if (endsWithIgnoreCase(folder, WEB_INF)) {
return FileVisitResult.SKIP_SUBTREE;
}
return FileVisitResult.CONTINUE;
}

@Override
public FileVisitResult visitFile(final Path file, final BasicFileAttributes attrs)
throws IOException {
final String fileName = file.getFileName().toString();
if (endsWithIgnoreCase(fileName, ".jsp") || endsWithIgnoreCase(fileName, ".jspx")) {
folders.add(file.getParent());
return FileVisitResult.SKIP_SIBLINGS;
}
return FileVisitResult.CONTINUE;
}

@Override
public FileVisitResult visitFileFailed(final Path file, final IOException exc)
throws IOException {
return FileVisitResult.CONTINUE;
}

@Override
public FileVisitResult postVisitDirectory(final Path dir, final IOException exc)
throws IOException {
return FileVisitResult.CONTINUE;
}
}
}
41 changes: 41 additions & 0 deletions dd-java-agent/agent-iast/src/main/java/com/datadog/iast/util/StringUtils.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package com.datadog.iast.util;

import javax.annotation.Nonnull;

public abstract class StringUtils {

private StringUtils() {}

/**
* Checks if the string ends with the selected suffix ignoring case. Note that this method does
* not take locale into account.
*/
public static boolean endsWithIgnoreCase(
@Nonnull final String value, @Nonnull final String suffix) {
if (value.length() < suffix.length()) {
return false;
}
if (suffix.isEmpty()) {
return true;
}
final int offset = value.length() - suffix.length();
return value.regionMatches(true, offset, suffix, 0, suffix.length());
}

/**
* Performs a substring of the selected string taking care of leading and trailing whitespaces.
*/
@Nonnull
public static String substringTrim(@Nonnull final String value, int start, int end) {
if (start >= end) {
return "";
}
while (start < end && Character.isWhitespace(value.charAt(start))) {
start++;
}
while (end > start && Character.isWhitespace(value.charAt(end - 1))) {
end--;
}
return start >= end ? "" : value.substring(start, end);
}
}
Loading

0 comments on commit 167e8f7

Please sign in to comment.