Skip to content

Commit 67795b6

Browse files
committed
HPCC-34575 Patch XSS injection of WsSMC/Activity ChatURL
Encode ChatURL value as per OWASP recommendation before returning. This prevents it from being executed as code when assigned to a JS variable, but does not prevent its use as a link elsewhere on the page. Recommended output encoding for JS contexts is to use the unicode `\uXXXX` hex code point format for all non-alphanumeric characters. Continue to strip out all nonprinting characaters as before. Signed-off-by: Terrence Asselin <terrence.asselin+copilot@lexisnexisrisk.com>
1 parent eb217ee commit 67795b6

2 files changed

Lines changed: 149 additions & 12 deletions

File tree

esp/services/ws_smc/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ include_directories (
6363
${CMAKE_BINARY_DIR}/oss
6464
${HPCC_SOURCE_DIR}/common/thorhelper
6565
${HPCC_SOURCE_DIR}/esp/espcommon
66+
${HPCC_SOURCE_DIR}/testing/unittests
6667
)
6768

6869
ADD_DEFINITIONS( -D_USRDLL -DWS_SMC_EXPORTS -DWSSMC_API_LOCAL -DESP_SERVICE_WsSMC)

esp/services/ws_smc/ws_smcService.cpp

Lines changed: 148 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333

3434
#include "roxiecontrol.hpp"
3535
#include "workunit.hpp"
36+
#include "junicode.hpp"
3637

3738
#define STATUS_SERVER_THOR "ThorMaster"
3839
#define STATUS_SERVER_HTHOR "HThorServer"
@@ -1031,23 +1032,158 @@ bool CWsSMCEx::onIndex(IEspContext &context, IEspSMCIndexRequest &req, IEspSMCIn
10311032
return true;
10321033
}
10331034

1034-
void CWsSMCEx::readBannerAndChatRequest(IEspContext& context, IEspActivityRequest &req, IEspActivityResponse& resp)
1035+
// Output encoding to include in a Javascript block.
1036+
// Follows OWASP guidance: keep [A-Za-z0-9], escape other printable ASCII as \uXXXX,
1037+
// properly escape non-ASCII by decoding UTF-8 first. Also drop ASCII control chars
1038+
// to maintain existing behavior.
1039+
StringBuffer& encodeJavascript(const char* value, StringBuffer& encoded)
10351040
{
1036-
StringBuffer chatURLStr, bannerStr;
1037-
const char* chatURL = req.getChatURL();
1038-
const char* banner = req.getBannerContent();
1039-
//Filter out invalid chars
1040-
if (chatURL && *chatURL)
1041+
if (isEmptyString(value))
1042+
return encoded;
1043+
1044+
const byte *p = reinterpret_cast<const byte *>(value);
1045+
const byte *end = p + strlen(value);
1046+
1047+
while (p < end)
10411048
{
1042-
const char* pStr = chatURL;
1043-
unsigned len = strlen(chatURL);
1044-
for (unsigned i = 0; i < len; i++)
1049+
UTF32 cp = readUtf8Character((unsigned)(end - p), p); // advances p safely
1050+
1051+
if (cp == sourceIllegal)
10451052
{
1046-
if (isprint(*pStr))
1047-
chatURLStr.append(*pStr);
1048-
pStr++;
1053+
encoded.append("\\uFFFD");
1054+
p++;
1055+
continue;
1056+
}
1057+
1058+
if (cp < 0x80)
1059+
{
1060+
// ASCII branch: preserve alnum, escape other printable ASCII, drop controls
1061+
unsigned char ch = static_cast<unsigned char>(cp);
1062+
if (isalnum(ch))
1063+
encoded.append(ch);
1064+
else if (isprint(ch))
1065+
encoded.appendf("\\u%04X", ch);
1066+
// else: non-printable ASCII -> omit
1067+
}
1068+
else if (cp <= 0xFFFF)
1069+
{
1070+
// BMP: escape as \uXXXX, but avoid lone surrogates
1071+
if (cp >= 0xD800 && cp <= 0xDFFF)
1072+
encoded.append("\\uFFFD");
1073+
else
1074+
encoded.appendf("\\u%04X", static_cast<unsigned>(cp));
10491075
}
1076+
else if (cp <= 0x10FFFF)
1077+
{
1078+
// Supplementary planes: escape as UTF-16 surrogate pair
1079+
unsigned u = static_cast<unsigned>(cp - 0x10000);
1080+
unsigned high = 0xD800 + (u >> 10);
1081+
unsigned low = 0xDC00 + (u & 0x3FF);
1082+
encoded.appendf("\\u%04X\\u%04X", high, low);
1083+
}
1084+
else
1085+
{
1086+
// Out of Unicode range
1087+
encoded.append("\\uFFFD");
1088+
}
1089+
1090+
if (cp == sourceExhausted)
1091+
break;
1092+
}
1093+
1094+
return encoded;
1095+
}
1096+
1097+
1098+
#ifdef _USE_CPPUNIT
1099+
1100+
#include "unittests.hpp"
1101+
1102+
class EncodeJavascriptTest : public CppUnit::TestFixture
1103+
{
1104+
CPPUNIT_TEST_SUITE( EncodeJavascriptTest );
1105+
CPPUNIT_TEST(testAsciiAlnum);
1106+
CPPUNIT_TEST(testAsciiPrintableEscaped);
1107+
CPPUNIT_TEST(testAsciiControlDropped);
1108+
CPPUNIT_TEST(testUtf8Bmp);
1109+
CPPUNIT_TEST(testUtf8Supplementary);
1110+
CPPUNIT_TEST(testInvalidUtf8);
1111+
CPPUNIT_TEST(testOutOfRange);
1112+
CPPUNIT_TEST_SUITE_END();
1113+
1114+
static void assertEncoded(const char *input, const char *expected)
1115+
{
1116+
StringBuffer out;
1117+
encodeJavascript(input, out);
1118+
ASSERT_EQUAL(std::string(out.str()), std::string(expected));
1119+
}
1120+
1121+
void testAsciiAlnum()
1122+
{
1123+
assertEncoded("Az09", "Az09");
1124+
}
1125+
1126+
void testAsciiPrintableEscaped()
1127+
{
1128+
// space, dash, underscore, dot should be escaped
1129+
assertEncoded(" -_.", "\\u0020\\u002D\\u005F\\u002E");
1130+
}
1131+
1132+
void testAsciiControlDropped()
1133+
{
1134+
StringBuffer in;
1135+
in.append('A').append((char)0x0A).append('B').append((char)0x09).append('C'); // A\nB\tC
1136+
assertEncoded(in.str(), "ABC");
1137+
}
1138+
1139+
void testUtf8Bmp()
1140+
{
1141+
// U+00E9 (é) -> \u00E9; U+4E2D (中) -> \u4E2D
1142+
StringBuffer in;
1143+
in.append(" "); // leading space to ensure escaping of ASCII printable
1144+
in.append((char)0xC3).append((char)0xA9); // é
1145+
in.append(" ");
1146+
in.append((char)0xE4).append((char)0xB8).append((char)0xAD); //
1147+
assertEncoded(in.str(), "\\u0020\\u00E9\\u0020\\u4E2D");
1148+
}
1149+
1150+
void testUtf8Supplementary()
1151+
{
1152+
// U+1F600 GRINNING FACE -> surrogate pair \uD83D\uDE00
1153+
StringBuffer in;
1154+
in.append((char)0xF0).append((char)0x9F).append((char)0x98).append((char)0x80);
1155+
assertEncoded(in.str(), "\\uD83D\\uDE00");
10501156
}
1157+
1158+
void testInvalidUtf8()
1159+
{
1160+
// Lone continuation byte 0x80 and truncated sequence 0xC2 at end -> each becomes \uFFFD
1161+
StringBuffer in;
1162+
in.append((char)0x80).append('X').append((char)0xC2);
1163+
assertEncoded(in.str(), "\\uFFFDX\\uFFFD");
1164+
}
1165+
1166+
void testOutOfRange()
1167+
{
1168+
// U+10FFFF is the highest Unicode code point
1169+
StringBuffer in;
1170+
in.append((char)0xF4).append((char)0x90).append((char)0x80).append((char)0x80); // U+110000
1171+
assertEncoded(in.str(), "\\uFFFD\\uFFFD\\uFFFD\\uFFFD");
1172+
}
1173+
};
1174+
1175+
CPPUNIT_TEST_SUITE_REGISTRATION( EncodeJavascriptTest );
1176+
CPPUNIT_TEST_SUITE_NAMED_REGISTRATION( EncodeJavascriptTest, "EncodeJavascriptTest" );
1177+
1178+
#endif // _USE_CPPUNIT
1179+
1180+
void CWsSMCEx::readBannerAndChatRequest(IEspContext& context, IEspActivityRequest &req, IEspActivityResponse& resp)
1181+
{
1182+
StringBuffer chatURLStr, bannerStr;
1183+
const char* chatURL = req.getChatURL();
1184+
const char* banner = req.getBannerContent();
1185+
//Filter out invalid chars and encode for safe assignment to javascript variable
1186+
encodeJavascript(chatURL, chatURLStr);
10511187
if (banner && *banner)
10521188
{
10531189
const char* pStr = banner;

0 commit comments

Comments
 (0)