False positive CWE-117 on sanitized user input in log message

[vyvy3]

Description & Reproduction

logger.warn(sanitized(responseInfo.replaceAll("[\r\n]+", "")));

Shows remediations as:

• Do not include unsanitized user input in log messages. This can allow attackers to manipulate log files or inject harmful content.

  String username = request.getParameter("username");
  log.warn("Username is" + username); // unsafe

• Do sanitize user input before logging it. Ensure that any data derived from user input is cleaned to prevent log injection attacks.

  String username = sanitized(request.getParameter("username"));
  log.warn("Username is" + username);

Expected Behavior

As the sanitized() method is called should pass

Actual Behavior

The same line failed the CRLF check and is fixed after adding replaceAll() above. My sanitized() method removes possible problematic symbols from the string and is mentioned below.

/**
	 * Sanitizes log message to prevent log injection attacks.
	 * @param input the log to sanitize.
	 * e
	 */
	public static String sanitized(String input) {
		if (input == null) {
			return "";
		}
		// Remove empty symbols
		String sanitizedInput = input.replaceAll("\\n\\r", "");

		// Remove non-printable characters
		sanitizedInput = sanitizedInput.replaceAll("[^\\p{Print}]", "");

		// Escape common special characters
		sanitizedInput = sanitizedInput
				.replace("\"", "\\\"")
				.replace("'", "\\'")
				.replace(";", "\\;");

		// Limit input size to avoid log bloating attacks
		if (sanitizedInput.length() > LOG_SANITIZED_INPUT_MAX_LENGTH) {
			sanitizedInput = sanitizedInput.substring(0, LOG_SANITIZED_INPUT_MAX_LENGTH) + "...";
		}

		return sanitizedInput;
	}

Possible Fix

Add wider scope what to check for in a string to prevent forgery and probably update the message in the report.

Your Environment

• Operating System and version:
• Output of 'bearer version':

bearer version 1.46.1
build 4ef7c0e9a1d2bf2c6c480a38bf13c1f6363af3fd
Docker image bearer/bearer:latest-amd64

[elsapet]

Hey @vyvy3 - thank you for raising this. Since Bearer is not a cross-function tool, it does not track the behaviour of custom methods such as sanitized() and therefore cannot determine whether the data has been adequately sanitized in this case. This is why it continues to raise the finding.

Here are your options to address this:

1. Mark findings as ignored, when your custom sanitized() method has been used. You could use the bearer ignore command to do this.
2. Create a custom CRLF rule, that acknowledges the sanitized() method as an adequate sanitizer, and then disable Bearer's own CRLF rule.