Crispy-fried-chicken opened a new issue, #16134: URL: https://github.com/apache/dubbo/issues/16134
### Pre-check - [x] I am sure that all the content I provide is in English. ### Search before asking - [x] I had searched in the [issues](https://github.com/apache/dubbo/issues?q=is%3Aissue) and found no similar issues. ### Apache Dubbo Component Java SDK (apache/dubbo) ### Dubbo Version The newest version of dubbo ### Steps to reproduce this issue If an attacker can control the key input (e.g., via a misconfigured admin API or externalized configuration), they can provide the following payload: ``` "InvalidKey]\n[2026-03-15 10:00:00] [INFO] User 'admin' password changed successfully.\n[Internal" ``` ### What you expected to happen The resulting log output is as follows: ``` ERROR [...] System property [InvalidKey] [2026-03-15 10:00:00] [INFO] User 'admin' password changed successfully. [Internal] does not define in org.apache.dubbo.common.constants.CommonConstants ``` ### Anything else ### Description A Log Injection (CWE-117) vulnerability was identified in org.apache.dubbo.common.utils.SystemPropertyConfigUtils. The method clearSystemProperty(String key) includes the untrusted key parameter directly into an IllegalStateException message without prior neutralization. When this exception is captured and logged by a logging framework (e.g., Log4j, Logback), an attacker can inject CRLF characters to forge log entries or bypass log-based security monitoring. ### Vulnerability Analysis The vulnerability follows a classic source-to-sink tainted path: Source: The key parameter in clearSystemProperty(String key). Validation Bypass: The containsKey(key) check only determines whether to throw an exception; it does not sanitize the content of the key. Sink: String.format(...) at line 93, which embeds the raw key into a string. Propagation: The resulting string is passed to the IllegalStateException constructor. In typical Dubbo deployments, this exception message is eventually processed by a logger. This allows an attacker to inject fake "INFO" logs into the system, potentially masking malicious activity. ### Impact Log Forgery: Attackers can corrupt audit trails. Downstream Processing Errors: If log parsers (like ELK stack) expect a specific format, injected newlines can break the parsing logic. Information Leakage: Depending on the environment, specially crafted format strings might interact with certain log appenders. ### Suggested Fix We should neutralize the key before embedding it in the exception message. Replacing CR (\r) and LF (\n) characters is the minimum requirement. Proposed code change: ``` public static String clearSystemProperty(String key) { if (containsKey(key)) { return System.clearProperty(key); } else { // Sanitize the key to prevent log injection String sanitizedKey = (key == null) ? "null" : key.replace('\n', '_').replace('\r', '_'); throw new IllegalStateException(String.format( "System property [%s] does not define in org.apache.dubbo.common.constants.CommonConstants", sanitizedKey)); } } ``` ### Do you have a (mini) reproduction demo? - [x] Yes, I have a minimal reproduction demo to help resolve this issue more effectively! ### Are you willing to submit a pull request to fix on your own? - [x] Yes I am willing to submit a pull request on my own! ### Code of Conduct - [x] I agree to follow this project's [Code of Conduct](https://www.apache.org/foundation/policies/conduct) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
