Skip to content

Commit 2878f61

Browse files
ar2rsawseenar2rsawseen
authored
Merge pull request #7555 from Countly/backport/fortify-fixes-24.05
[security][backport 24.05] Fortify-flagged path traversal, XSS, and info-leak fixes
2 parents a163fde + d9c9dcb commit 2878f61

5 files changed

Lines changed: 41 additions & 15 deletions

File tree

‎api/utils/common.js‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1562,7 +1562,10 @@ common.returnRaw = function(params, returnCode, body, heads) {
15621562
else {
15631563
console.error("Output already closed, can't write more");
15641564
console.trace();
1565-
console.log(params);
1565+
// Don't dump the full params object — req.body/req.headers can
1566+
// contain credentials, session cookies, or other secrets. Log
1567+
// only the pathname (query string can carry api_key/auth_token).
1568+
console.log({pathname: params.urlParts && params.urlParts.pathname, apiPath: params.apiPath, qstringKeys: params.qstring && Object.keys(params.qstring)});
15661569
}
15671570
}
15681571
};
@@ -1625,7 +1628,10 @@ common.returnMessage = function(params, returnCode, message, heads, noResult = f
16251628
else {
16261629
console.error("Output already closed, can't write more");
16271630
console.trace();
1628-
console.log(params);
1631+
// Don't dump the full params object — req.body/req.headers can
1632+
// contain credentials, session cookies, or other secrets. Log
1633+
// only the pathname (query string can carry api_key/auth_token).
1634+
console.log({pathname: params.urlParts && params.urlParts.pathname, apiPath: params.apiPath, qstringKeys: params.qstring && Object.keys(params.qstring)});
16291635
}
16301636
}
16311637
};
@@ -1703,7 +1709,10 @@ common.returnOutput = function(params, output, noescape, heads) {
17031709
else {
17041710
console.error("Output already closed, can't write more");
17051711
console.trace();
1706-
console.log(params);
1712+
// Don't dump the full params object — req.body/req.headers can
1713+
// contain credentials, session cookies, or other secrets. Log
1714+
// only the pathname (query string can carry api_key/auth_token).
1715+
console.log({pathname: params.urlParts && params.urlParts.pathname, apiPath: params.apiPath, qstringKeys: params.qstring && Object.keys(params.qstring)});
17071716
}
17081717
}
17091718
};

‎frontend/express/app.js‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -439,14 +439,29 @@ Promise.all([plugins.dbConnection(countlyConfig), plugins.dbConnection("countly_
439439
app.use(cookieParser());
440440
//server theme images
441441
app.use(function(req, res, next) {
442-
var urlPath = req.url.replace(countlyConfig.path, "");
442+
var urlPath = req.path.replace(countlyConfig.path, "");
443443
var theme = req.cookies.theme || curTheme;
444-
if (theme && theme.length && (req.url.indexOf(countlyConfig.path + '/images/') === 0 || req.url.indexOf(countlyConfig.path + '/geodata/') === 0)) {
445-
fs.exists(__dirname + '/public/themes/' + theme + urlPath, function(exists) {
446-
if (exists) {
447-
res.sendFile(__dirname + '/public/themes/' + theme + urlPath);
444+
if (theme && theme.length && (req.path.indexOf(countlyConfig.path + '/images/') === 0 || req.path.indexOf(countlyConfig.path + '/geodata/') === 0)) {
445+
// The `theme` cookie is user-controlled. Restrict it to a plain
446+
// filename (no separators, no leading dots, no nulls) before
447+
// building the path. This is defense-in-depth on top of the
448+
// `root` option below; reject anything that doesn't survive
449+
// sanitizeFilename unchanged.
450+
if (common.sanitizeFilename(theme) !== theme) {
451+
next();
452+
return;
453+
}
454+
// Hand the relative path to res.sendFile with `root` set to
455+
// /public/themes — express normalizes the path and rejects any
456+
// `..` traversal before touching the filesystem. Missing files
457+
// and traversal-blocked requests fall through to next(); other
458+
// errors are logged server-side but still fall through so a
459+
// theme misconfiguration doesn't 500 the page.
460+
res.sendFile(theme + urlPath, {root: path.resolve(__dirname, 'public/themes')}, function(err) {
461+
if (err && err.code !== 'ENOENT' && err.statusCode !== 403 && err.statusCode !== 404) {
462+
log.e('Error serving theme image %j: %s', req.path, err.message);
448463
}
449-
else {
464+
if (err) {
450465
next();
451466
}
452467
});

‎plugins/sdk/api/api.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@ plugins.register("/permissions/features", function(ob) {
2727
common.returnOutput(params, config);
2828
})
2929
.catch(function(err) {
30-
common.returnMessage(params, 400, 'Error: ' + err);
30+
console.error("Error retrieving SDK config", err);
31+
common.returnMessage(params, 400, 'Error retrieving SDK config');
3132
})
3233
.finally(function() {
3334
resolve();
@@ -72,7 +73,8 @@ plugins.register("/permissions/features", function(ob) {
7273
common.returnOutput(params, res.config || {});
7374
})
7475
.catch(function(err) {
75-
common.returnMessage(params, 400, 'Error: ' + err);
76+
console.error("Error retrieving SDK config", err);
77+
common.returnMessage(params, 400, 'Error retrieving SDK config');
7678
});
7779
});
7880

‎plugins/two-factor-auth/frontend/public/templates/enter2fa_login.html‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,8 @@
8888
<%- inject_template.form %>
8989
<% } %>
9090
<div>
91-
<input type="hidden" value="<%- username %>" name="username"/>
92-
<input type="hidden" value="<%- password %>" name="password"/>
91+
<input type="hidden" value="<%= username %>" name="username"/>
92+
<input type="hidden" value="<%= password %>" name="password"/>
9393
<input type="hidden" value="<%= csrf %>" name="_csrf" />
9494
<input type="hidden" value="en" name="lang" id="form-lang" />
9595
<input id="login-button" value="Continue" type="submit" data-localize="two-factor-auth.continue"/>

‎plugins/two-factor-auth/frontend/public/templates/setup2fa.html‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,8 @@
8181
<% } %>
8282
<div>
8383
<input type="hidden" value="<%= secret_token %>" name="secret_token"/>
84-
<input type="hidden" value="<%- username %>" name="username"/>
85-
<input type="hidden" value="<%- password %>" name="password"/>
84+
<input type="hidden" value="<%= username %>" name="username"/>
85+
<input type="hidden" value="<%= password %>" name="password"/>
8686
<input type="hidden" value="<%= csrf %>" name="_csrf" />
8787
<input type="hidden" value="en" name="lang" id="form-lang" />
8888
</div>

0 commit comments

Comments
 (0)