Summary: | Log Forging | ||
---|---|---|---|
Product: | POI | Reporter: | Donald <dkwakkel> |
Component: | POI Overall | Assignee: | POI Developers List <dev> |
Status: | RESOLVED FIXED | ||
Severity: | normal | ||
Priority: | P2 | ||
Version: | unspecified | ||
Target Milestone: | --- | ||
Hardware: | PC | ||
OS: | Linux |
Description
Donald
2015-06-16 12:51:46 UTC
Here are two links to this topic from the first google matches ...: the owasp entry where the entry text was copied from [1] and how to setup fortify [2] to create custom rules for it I haven't checked our sonarcube instance for this error and we somehow don't get a slot in the apache sonarcube. Maybe some can provide a current fortify (or similar) report for this error? Andi. [1] https://www.owasp.org/index.php/Log_Forging [2] http://stackoverflow.com/questions/12784707/log-forging-fortify-fix Hi Andi, It is not clear to me if you need INFO from me or someone else. See this article how to solve log forging (just replace CR/LF): http://www.jtmelton.com/2010/09/21/preventing-log-forging-in-java/ If you need more info from me, please let me know. Actually I'm not sure how to fix this ...: first thought was, there might be a config option in the underlying logger, but we can't rely on it as we have different logger interfaces which some (or all?) do not provide such an option. Next thought was, to simply change the POILogger class and sanitize the CR/LFs, limit the length, but then we also might need XSS filtering. I don't like the idea of forcing html encoding in the logging class, just because the log might be viewed in a browser. So maybe we just provide another logging facade with the above features, but then should we enable it by default, by system property (which nobody realize to set it) or some heuristic ("we are running in an appserver, so we should activate xss filtering, because appserver logs are often viewed online ...")? Andi I think something like XSS is thevresponsibility of the web app doing the displaying, not the logger. At least I did not come across projects which do this in such a pro-active way. I would just do the CR/LF replacing for now always as additional options/properties tend to be ignored by users :) Fixed with r1686748 The fix just replaces CR/LF - the exception logging stays unchanged. I've changed the log methods to varargs and because this leads to binary incompatibilities, marked the classes as internal, as I think user code shouldn't depend on our internal log propagation mechanism. Depending on the discussion [1], we might eventually include and move to SLF4J logging. Andi. [1] http://apache-poi.1045710.n5.nabble.com/Logging-Binary-compatibility-td5719152.html |