Security Alert from GitHub for "Incomplete string escaping or encoding"

Scheduler Javascript
#1

GitHub created a security alert because I embed the DHTMLX Scheduler code. I track it in this issue:

I also wanted to report this here for conversation and also to know what you think about its seriousness.
What are your thoughts on this?

#2

Hi!

Thank you for bringing this to our attention!

This security alert points at this line:

var code = "return \"" + (els[i].innerHTML || "").replace(/"/g, "\\\"").replace(/[\n\r]+/g, "") + "\";";

which is located in html_templates plugin Full List of Extensions Scheduler Docs .

And the code scan shows the following message:

This does not escape backslash characters in the input.

Here is what happens there - the code in question parses HTML templates from the markup (a feature of Scheduler that is rarely used) and generates template functions from them.
In the process the code escapes all quote characters and removes all newline characters from the template, as such characters can conflict with the code of function that is generated. The warning says that the code doesn’t escape backlash characters (\) which can exist in html and form an escape sequence such as newline character (\n) or tab (\t).

Looks like we’ve indeed missed that replacement and correct it, it will be included in the upcoming update of the Scheduler.

However, the issue itself seems minor and can cause either an incorrect display of HTML templates which contain backlash characters or a runtime script error. HTML templates themselves are part of application code that is written by the developer and not formed from user input, so it’s unlikely that unescaped characters there can be exploited in any way.

From the security perspective this issue doesn’t introduce any security threats since the component doesn’t have any specific built-in measures for preventing XSS/CSRF/SQL Injection attacks. The assumption is the value escaping is supposed to be done at the application level and at the backend by the end developer: Application Security Scheduler Docs
The code in question performed replacements specifically to ensure correct display of template value, so even working as intended (replacing backlashes) it is not designed to prevent any vulnerabilities.

To summarize:

  1. As we see it now, it’s not a security issue
  2. It potentially can cause the incorrect work of html templates plugin
  3. We’ll correct it in the next update

Again, thank you for informing us!

1 Like