Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion WebContent/WEB-INF/dwr.xml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
You should have received a copy of the GNU General Public License
along with this program. If not, see http://www.gnu.org/licenses/.
-->
<!DOCTYPE dwr PUBLIC "-//GetAhead Limited//DTD Direct Web Remoting 2.0//EN" "http://getahead.org/dwr/dwr20.dtd">
<!DOCTYPE dwr PUBLIC "-//GetAhead Limited//DTD Direct Web Remoting 3.0//EN" "http://getahead.org/dwr/dwr30.dtd">
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

DWR 3.0 Direct Web Remoting dwr.xml DOCTYPE DTD reference specification

💡 Result:

Use this DOCTYPE in dwr.xml for DWR 3.0:

(Older examples also reference "http://getahead.org/dwr/dwr30.dtd" with the same public id.) [1][2]


🌐 Web query:

getahead.org dwr30.dtd external DTD remote fetch best practices

💡 Result:

Short answer: avoid letting XML parsers fetch external DTDs (like getahead.org/dwr30.dtd) from the network. Either disable external DTD/ENTITY loading or resolve DTDs locally and tightly control when/what the parser may fetch. Key practices:

  • Disable DTDs or external-entity resolution in your parser (best): disallow DOCTYPE, disable external entities and external DTD loading. [1][2][3]
  • If DTDs are required, use a local, versioned copy (vendor bundle or internal repository) and configure the parser to resolve only that local store (catalog resolver/whitelist). Do not permit arbitrary HTTP fetches. [6][2]
  • Set parser attributes to block network access: e.g. set XMLConstants.ACCESS_EXTERNAL_DTD and ACCESS_EXTERNAL_SCHEMA to "" or set apache feature http://apache.org/xml/features/nonvalidating/load-external-dtd to false. Also enable secure-processing features. [2][3][4]
  • Harden entity handling: disable entity expansion or limit expansions to prevent Billion‑Laughs/DoS. [1][3]
  • Network/operational controls: block outbound HTTP(S) from parse-time processes (egress rules) or allow only trusted hosts; log/alert any attempts to fetch external DTDs. [1][5]
  • Input validation and safe libraries: treat incoming XML as untrusted, validate against schemas that do not require external fetches and prefer parsers/libraries built with XXE protections (or use JSON instead if feasible). [1][3][5]
  • Fail-safe defaults and testing: audit code for DOCTYPE/entity handling, add unit tests and security tests that exercise XXE/SSRF and remote-DTD cases. Monitor dependencies for references to remote DTDs like getahead.org/dwr30.dtd and replace them. [1][4]

Sources: OWASP XML-External-Entity cheat sheet; Oracle Java XML guidance; Semgrep/secure-coding guidance; SecureCodingPractices article; general DTD pitfalls. [1][2][3][4][5]

References
[1] OWASP XML External Entity Prevention Cheat Sheet.
[2] Oracle — Java Technology and XML (reduce external entity/DTD cost and controls).
[3] Semgrep / XML Security guidance (disable DTDs / external entities).
[4] SecureCodingPractices — Avoid XXE in Java parsers (features/attributes examples).
[5] Article on DTD pitfalls and availability/performance/security trade-offs.


Disable external DTD loading or use local DTD resolution.
While the PUBLIC identifier is correct for DWR 3.0, fetching the DTD from an external URL (http://getahead.org/dwr/dwr30.dtd) introduces security risks (XXE/SSRF vulnerabilities) and operational fragility (network failures, offline/restricted environments). Configure your XML parser to disable external DTD/entity loading, or replace with a local, versioned copy of the DTD.

🤖 Prompt for AI Agents
In @WebContent/WEB-INF/dwr.xml at line 19, The DOCTYPE line in dwr.xml
references an external DTD ("-//GetAhead Limited//DTD Direct Web Remoting
3.0//EN" "http://getahead.org/dwr/dwr30.dtd") which can cause XXE/SSRF and
runtime failures; update your configuration to either (a) remove the external
system URL and point the DOCTYPE to a bundled local copy of dwr30.dtd included
with the application, or (b) keep the PUBLIC id but configure your XML
parser/Factory to disable external entity/DTD loading and enable a local
EntityResolver/resource resolver that returns the bundled dwr30.dtd; ensure the
change targets the XML parsing code that reads dwr.xml (or the deployment
parser) so external fetching is prevented.


<dwr>
<init>
Expand Down
Binary file modified WebContent/WEB-INF/lib/dwr.jar
Binary file not shown.
19 changes: 19 additions & 0 deletions WebContent/resources/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,25 @@ var mango = {};

var lasTimeUpdate;

// DWR 2 compatibility: global $() alias.
(function () {
if (typeof window.$ === 'function') {
return;
}

if (window.dwr && dwr.util && typeof dwr.util.byId === 'function') {
window.$ = dwr.util.byId;
return;
}

window.$ = function (id) {
if (typeof id === 'string') {
return document.getElementById(id);
}
return id;
};
})();

//
// String prototypes
//
Expand Down
4 changes: 2 additions & 2 deletions src/com/serotonin/mango/web/dwr/OpcUaDataTypeConverter.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
public class OpcUaDataTypeConverter extends EnumConverter {

@Override
public Object convertInbound(Class paramType, InboundVariable iv, InboundContext inctx) throws MarshallException {
String value = LocalUtil.decode(iv.getValue());
public Object convertInbound(Class paramType, InboundVariable iv) throws MarshallException {
String value = LocalUtil.urlDecode(iv.getValue());
try {
return OpcUaDataType.valueByNameOf(value);
} catch (Exception var9) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
public class ProtocolVersionConverter extends EnumConverter {

@Override
public Object convertInbound(Class paramType, InboundVariable iv, InboundContext inctx) throws MarshallException {
String value = LocalUtil.decode(iv.getValue());
public Object convertInbound(Class paramType, InboundVariable iv) throws MarshallException {
String value = LocalUtil.urlDecode(iv.getValue());
try {
return ProtocolVersion.protocolVersion(value);
} catch (Exception var9) {
Expand Down
28 changes: 10 additions & 18 deletions src/com/serotonin/mango/web/filter/WebContextFilter.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,41 +23,33 @@
import javax.servlet.Filter;
import javax.servlet.FilterChain;
import javax.servlet.FilterConfig;
import javax.servlet.ServletContext;
import javax.servlet.ServletException;
import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import org.directwebremoting.impl.DefaultWebContextBuilder;
import org.directwebremoting.servlet.DwrWebContextFilter;

/**
* @author Matthew Lohbihler
*/
public class WebContextFilter implements Filter {
private final DefaultWebContextBuilder builder = new DefaultWebContextBuilder();
private ServletContext servletContext;

private final DwrWebContextFilter delegate = new DwrWebContextFilter();


@Override
public void init(FilterConfig config) {
servletContext = config.getServletContext();
public void init(FilterConfig config) throws ServletException {
delegate.init(config);
}

@Override
public void destroy() {
// no op
delegate.destroy();
}

@Override
public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException,
ServletException {
try {
builder.set((HttpServletRequest) request, (HttpServletResponse) response, null, servletContext, null);
chain.doFilter(request, response);
}
finally {
builder.unset();
}
public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
throws IOException, ServletException {
delegate.doFilter(request, response, chain);
}
}
167 changes: 167 additions & 0 deletions src/com/serotonin/web/taglib/DwrConvertTag.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
package com.serotonin.web.taglib;

import javax.servlet.jsp.JspException;
import javax.servlet.jsp.JspWriter;
import javax.servlet.jsp.tagext.TagSupport;
import javax.servlet.http.HttpServletRequest;
import java.io.IOException;
import java.lang.reflect.Array;
import java.util.Collection;
import java.util.Iterator;
import java.util.Map;
import java.util.ResourceBundle;

import org.apache.logging.log4j.core.util.JsonUtils;
import com.serotonin.web.i18n.I18NUtils;
import com.serotonin.web.i18n.LocalizableMessage;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Check for project dependencies - look for pom.xml or gradle.build
git ls-files | grep -E "(pom\.xml|build\.gradle|ivy\.xml)" | head -5

Repository: SCADA-LTS/Scada-LTS

Length of output: 100


🏁 Script executed:

# Check the full DwrConvertTag.java file
find . -name "DwrConvertTag.java" -type f

Repository: SCADA-LTS/Scada-LTS

Length of output: 112


🏁 Script executed:

# Check for taglib descriptor files
git ls-files | grep -E "\.(tld|xml)$" | grep -i tag | head -10

Repository: SCADA-LTS/Scada-LTS

Length of output: 45


🏁 Script executed:

# Check main build.gradle for log4j-core dependency
cat build.gradle | grep -A 50 -B 5 -i "log4j"

Repository: SCADA-LTS/Scada-LTS

Length of output: 45


🏁 Script executed:

# Check the full DwrConvertTag.java file
cat -n src/com/serotonin/web/taglib/DwrConvertTag.java

Repository: SCADA-LTS/Scada-LTS

Length of output: 5900


🏁 Script executed:

# Read the full build.gradle to check dependencies
cat build.gradle | head -200

Repository: SCADA-LTS/Scada-LTS

Length of output: 6341


🏁 Script executed:

# Search for any mention of log4j or json libraries more broadly
rg "log4j|jackson|gson|json-p|org\.json" build.gradle

Repository: SCADA-LTS/Scada-LTS

Length of output: 45


🏁 Script executed:

# Check if there's any usage of JsonUtils in the codebase
rg "JsonUtils" --type java

Repository: SCADA-LTS/Scada-LTS

Length of output: 570


🏁 Script executed:

# Check what's in WEB-INF/lib to see if log4j-core is there
find WebContent/WEB-INF/lib -name "*log4j*" 2>/dev/null | head -20

Repository: SCADA-LTS/Scada-LTS

Length of output: 291


🏁 Script executed:

# Check the lib directory as well
find lib -name "*log4j*" 2>/dev/null | head -20

Repository: SCADA-LTS/Scada-LTS

Length of output: 45


🏁 Script executed:

# Search for any JSON-related libraries in the project that might be better options
find WebContent/WEB-INF/lib lib -name "*.jar" 2>/dev/null | grep -iE "(jackson|gson|json)" | head -20

Repository: SCADA-LTS/Scada-LTS

Length of output: 484


🏁 Script executed:

# Check if there are any .tld files at all
find . -name "*.tld" -o -name "*.xml" | grep -i taglib 2>/dev/null | head -10

Repository: SCADA-LTS/Scada-LTS

Length of output: 45


🏁 Script executed:

# Verify JsonUtils.quoteAsString escaping by checking log4j source or docs
# Check if there are any tests or usages that show what characters it escapes
rg "quoteAsString" -A 5 -B 5 --type java

Repository: SCADA-LTS/Scada-LTS

Length of output: 783


🏁 Script executed:

# Check if Jackson ObjectMapper is used elsewhere in the codebase
rg "ObjectMapper|Jackson|writeValueAsString" --type java | head -15

Repository: SCADA-LTS/Scada-LTS

Length of output: 2305


Replace log4j-core's internal JsonUtils with Jackson and fix Number serialization.

The project includes log4j-core-2.16.0.jar and jackson-databind-2.17.2.jar. Using JsonUtils.quoteAsString() is problematic because:

  1. It's an internal API (org.apache.logging.log4j.core.util.*), not part of log4j's public contract, while Jackson is already the established pattern in the codebase (see SystemSettingsUtils and test usage).
  2. Number.toString() (line 57–59) emits invalid JSON: values like NaN, Infinity, and -Infinity are not valid JSON numbers.
  3. Limited HTML-safe escaping: The current escaping does not encode <, >, or &, risking XSS if JSON is embedded in HTML/script contexts without extra precautions.

Use Jackson's ObjectMapper (already integrated and available via the project's dependency chain). Jackson correctly rejects or safely handles NaN/Infinity, and allows configuration of proper JSON escaping for both syntax and HTML contexts.

🤖 Prompt for AI Agents
In @src/com/serotonin/web/taglib/DwrConvertTag.java around lines 14 - 16,
Replace usage of the internal log4j JsonUtils.quoteAsString and raw
Number.toString in DwrConvertTag by wiring in Jackson's ObjectMapper:
create/configure a shared ObjectMapper (or use existing project one) and call
writeValueAsString(...) to produce JSON strings instead of manual quoting;
before serializing Numbers, detect Float/Double and if
Double.isNaN/Double.isInfinite (or Float equivalents) handle them
deterministically (e.g., serialize as null or throw a clear
IllegalArgumentException) so invalid JSON tokens like NaN/Infinity are not
emitted; also configure the ObjectMapper/JsonFactory with a CharacterEscapes
implementation (or use StdSerializer that escapes HTML-sensitive chars) to
ensure '<', '>', '&' are escaped for safe embedding in HTML/script; finally
remove imports of org.apache.logging.log4j.core.util.JsonUtils and replace all
calls in DwrConvertTag (the methods that previously used JsonUtils.quoteAsString
and Number.toString) with the new ObjectMapper-based serialization and the
NaN/Infinity guard.


public class DwrConvertTag extends TagSupport {

private Object obj;

public DwrConvertTag() {
// no-op
}

public void setObj(Object obj) {
this.obj = obj;
}

@Override
public int doStartTag() throws JspException {
try {
JspWriter out = pageContext.getOut();
String json = toJson(obj);
out.write(json);
return SKIP_BODY;
} catch (IOException e) {
throw new JspException("Error writing DwrConvertTag JSON content", e);
}
}

@Override
public void release() {
super.release();
this.obj = null;
}

Comment on lines 21 to 52
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find tag registration and usage
rg -n --fixed-strings "DwrConvertTag" -C2
rg -n --glob='**/*.tld' -C2 "com.serotonin.web.taglib.DwrConvertTag"
rg -n --glob='**/*.jsp' -C2 "dwrConvert" "DwrConvertTag"

Repository: SCADA-LTS/Scada-LTS

Length of output: 1917


🏁 Script executed:

#!/bin/bash
# View the full TLD descriptor entry for the convert tag
cat -n WebContent/WEB-INF/tld/escape.tld | sed -n '550,570p'

# Search for actual usage in JSP files
fd --type f -e jsp -e jspx | head -20 | xargs -I {} rg -l "convert" {} 2>/dev/null

Repository: SCADA-LTS/Scada-LTS

Length of output: 839


🏁 Script executed:

#!/bin/bash
# Find JSP files and search for the convert tag usage
# The tag would typically be used with a namespace prefix like <escape:convert ...>
find . -name "*.jsp" -o -name "*.jspx" | xargs rg -l "convert" 2>/dev/null | head -10
# Also search explicitly for the tag pattern
rg -n "escape:convert|<convert" --type jsp 2>/dev/null | head -20

Repository: SCADA-LTS/Scada-LTS

Length of output: 420


🏁 Script executed:

#!/bin/bash
# Search for escape:convert tag usage in JSP files
rg -n "escape:convert" --type jsp 2>/dev/null

# Also check for the tag declaration in JSP files
rg -n "<%@ taglib.*escape" --type jsp 2>/dev/null | head -5

Repository: SCADA-LTS/Scada-LTS

Length of output: 45


🏁 Script executed:

#!/bin/bash
# Check if any JSP files actually import the escape taglib
rg -n "taglib.*escape" --type jsp 2>/dev/null | head -10

# Search for any test files or usage
find . -name "*Test*.java" -o -name "*test*.java" | xargs rg -l "DwrConvertTag\|escape:convert" 2>/dev/null

# Check for documentation or examples
find . -name "*.md" -o -name "*.txt" -o -name "*.rst" | xargs rg -l "DwrConvertTag\|dwrConvert" 2>/dev/null

Repository: SCADA-LTS/Scada-LTS

Length of output: 47


🏁 Script executed:

#!/bin/bash
# Simpler search for taglib imports
rg "taglib" --type jsp 2>/dev/null | grep -i escape | head -10

# Check for any JSP importing the escape TLD
rg -n "escape.tld\|uri=\".*escape" --type jsp 2>/dev/null | head -10

# Count occurrences of "convert" in JSP files to understand context
rg -n "convert" --type jsp 2>/dev/null | head -20

Repository: SCADA-LTS/Scada-LTS

Length of output: 45


🏁 Script executed:

#!/bin/bash
# Check web.xml for TLD references
rg -n "escape.tld" --type xml 2>/dev/null

# Check if there's a web.xml file
find . -name "web.xml" -type f | head -3

# Look for taglib-uri declarations
rg -n "taglib-uri" --type xml 2>/dev/null | head -20

# Check escape.tld header to see URI
head -20 WebContent/WEB-INF/tld/escape.tld

Repository: SCADA-LTS/Scada-LTS

Length of output: 963


🏁 Script executed:

#!/bin/bash
# List all TLD files
ls -la WebContent/WEB-INF/tld/

# Verify which file contains the convert tag
rg -l "convert" WebContent/WEB-INF/tld/

Repository: SCADA-LTS/Scada-LTS

Length of output: 335


🏁 Script executed:

#!/bin/bash
# Check if escape.tld is referenced in web.xml
cat WebContent/WEB-INF/web.xml | grep -i "escape\|serotonin" | head -20

# Check the URI declared in escape.tld
head -15 WebContent/WEB-INF/tld/escape.tld | grep -i uri

# Search for JSP imports with the URI pattern
rg -n "http://www.serotoninsoftware.com" --type jsp 2>/dev/null

Repository: SCADA-LTS/Scada-LTS

Length of output: 1757


🏁 Script executed:

#!/bin/bash
# Check escape.tld for its declared URI
grep -n "uri" WebContent/WEB-INF/tld/escape.tld | head -3

# Search JSP files for taglib imports with escape or serotonin URIs
rg -n "<%@ taglib" --type jsp 2>/dev/null | head -30

Repository: SCADA-LTS/Scada-LTS

Length of output: 118


🏁 Script executed:

#!/bin/bash
# Search for taglib directives in JSP using different pattern
rg "<%@.*taglib" --type jsp 2>/dev/null | head -20

# Also search for URI mappings in web.xml
grep -n "taglib-uri\|taglib-location" WebContent/WEB-INF/web.xml 2>/dev/null

Repository: SCADA-LTS/Scada-LTS

Length of output: 45


🏁 Script executed:

#!/bin/bash
# Final check: search for any reference to "dwrConvert" or "dwr" that might indicate usage
rg -i "dwr" --type jsp --type java 2>/dev/null | head -20

# Also check comments in the TLD
grep -A 2 -B 2 "convert" WebContent/WEB-INF/tld/escape.tld

Repository: SCADA-LTS/Scada-LTS

Length of output: 215


The tag IS properly registered in the TLD but appears unused in JSP pages.
The tag is correctly wired in WebContent/WEB-INF/tld/escape.tld (lines 552–561) with the name convert, required obj attribute, and proper configuration. However, no usage of this tag (e.g., <escape:convert> or imports of the escape taglib) was found in any JSP files. If this tag is genuinely needed, add JSP usage examples or documentation specifying its output context (e.g., only within <script type="application/json"> blocks). If unused, consider removing it.

🤖 Prompt for AI Agents
In @src/com/serotonin/web/taglib/DwrConvertTag.java around lines 18 - 47, The
DwrConvertTag (class DwrConvertTag) is declared in the TLD as tag name "convert"
but no JSP usage was found; either add example usages/docs showing where to use
<escape:convert> (e.g., demonstrate import of the escape taglib and use within a
<script type="application/json"> or other appropriate context) and a short JSP
snippet in the project docs or a sample JSP, or remove the tag and its TLD entry
if truly unused; update the TLD (escape.tld) and project docs or delete the
DwrConvertTag class accordingly to keep code and configuration in sync.

private String toJson(Object value) {
if (value == null) {
return "null";
}

if (value instanceof Boolean) {
return ((Boolean) value) ? "true" : "false";
}

if (value instanceof Number) {
return value.toString();
}

if (value instanceof CharSequence) {
return quoteString(value.toString());
}

if (value instanceof LocalizableMessage) {
String localized = resolveLocalizableMessage((LocalizableMessage) value);
return quoteString(localized);
}

if (value instanceof Map<?, ?>) {
return mapToJson((Map<?, ?>) value);
}

if (value instanceof Collection<?>) {
return collectionToJson((Collection<?>) value);
}

if (value.getClass().isArray()) {
return arrayToJson(value);
}

return quoteString(String.valueOf(value));
}

private String resolveLocalizableMessage(LocalizableMessage lm) {
try {
if (pageContext == null) {
return lm.getLocalizedMessage(null);
}

Object req = pageContext.getRequest();
if (req instanceof HttpServletRequest) {
HttpServletRequest httpReq = (HttpServletRequest) req;
ResourceBundle bundle = I18NUtils.getBundle(httpReq);
return lm.getLocalizedMessage(bundle);
}

return lm.getLocalizedMessage(null);
} catch (Exception e) {
String key = lm.getKey();
return key != null ? key : lm.serialize();
}
}

private String mapToJson(Map<?, ?> map) {
StringBuilder sb = new StringBuilder();
sb.append('{');
Iterator<? extends Map.Entry<?, ?>> it = map.entrySet().iterator();
boolean first = true;
while (it.hasNext()) {
Map.Entry<?, ?> entry = it.next();
if (!first) {
sb.append(',');
}
first = false;

String key = String.valueOf(entry.getKey());
Object val = entry.getValue();

sb.append(quoteString(key));
sb.append(':');
sb.append(toJson(val));
}
sb.append('}');
return sb.toString();
}

private String collectionToJson(Collection<?> collection) {
StringBuilder sb = new StringBuilder();
sb.append('[');
Iterator<?> it = collection.iterator();
boolean first = true;
while (it.hasNext()) {
if (!first) {
sb.append(',');
}
first = false;
Object val = it.next();
sb.append(toJson(val));
}
sb.append(']');
return sb.toString();
}

private String arrayToJson(Object array) {
StringBuilder sb = new StringBuilder();
sb.append('[');
int length = Array.getLength(array);
for (int i = 0; i < length; i++) {
if (i > 0) {
sb.append(',');
}
Object val = Array.get(array, i);
sb.append(toJson(val));
}
sb.append(']');
return sb.toString();
}

private String quoteString(String s) {
StringBuilder sb = new StringBuilder(s.length() + 16);
sb.append('\"');
JsonUtils.quoteAsString(s, sb);
sb.append('\"');
return sb.toString();
}
}
Loading