Skip to content

Commit 924d25c

Browse files
committed
fix: html-escape debug data sent to development-only interactions
closes #1414 Signed-off-by: Filip Skokan <panva.ip@gmail.com>
1 parent 123b3af commit 924d25c

6 files changed

Lines changed: 52 additions & 3 deletions

File tree

SECURITY.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,3 +97,4 @@ The following are explicitly **not** considered vulnerabilities in this library:
9797
- **Denial of service via resource exhaustion** ([CWE-400](https://cwe.mitre.org/data/definitions/400.html)): While the library validates inputs, it does not implement resource limits. Applications should implement their own rate limiting and resource management.
9898
- **Misconfiguration**: Security issues arising from insecure configuration choices (e.g., weak policies, insecure client settings) are the user's responsibility.
9999
- **Insecure adapter implementations**: Security issues in user-provided storage adapters are the user's responsibility.
100+
- **Development-only features**: Bugs or issues in features explicitly documented as development-only (e.g., `features.devInteractions`, the built-in development-only signing keys used when no `jwks` is configured, the in-memory adapter) are not considered vulnerabilities. These features are intended solely for development and demonstration purposes and must not be used in production deployments.

example/routes/express.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import isEmpty from 'lodash/isEmpty.js';
77
import { urlencoded } from 'express';
88

99
import Account from '../support/account.js';
10+
import htmlSafe from '../support/html_safe.js';
1011
import { errors } from '../../lib/index.js'; // from 'oidc-provider';
1112

1213
const body = urlencoded({ extended: false });
@@ -18,7 +19,7 @@ const debug = (obj) => querystring.stringify(Object.entries(obj).reduce((acc, [k
1819
acc[key] = inspect(value, { depth: null });
1920
return acc;
2021
}, {}), '<br/>', ': ', {
21-
encodeURIComponent(value) { return keys.has(value) ? `<strong>${value}</strong>` : value; },
22+
encodeURIComponent(value) { return keys.has(value) ? `<strong>${value}</strong>` : htmlSafe(value); },
2223
});
2324
const { SessionNotFound } = errors;
2425
export default (app, provider) => {

example/routes/koa.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import Router from '@koa/router';
1111

1212
import { defaults } from '../../lib/helpers/defaults.js'; // make your own, you'll need it anyway
1313
import Account from '../support/account.js';
14+
import htmlSafe from '../support/html_safe.js';
1415
import { errors } from '../../lib/index.js'; // from 'oidc-provider';
1516

1617
const hkdf = promisify(crypto.hkdf);
@@ -21,7 +22,7 @@ const debug = (obj) => querystring.stringify(Object.entries(obj).reduce((acc, [k
2122
acc[key] = inspect(value, { depth: null });
2223
return acc;
2324
}, {}), '<br/>', ': ', {
24-
encodeURIComponent(value) { return keys.has(value) ? `<strong>${value}</strong>` : value; },
25+
encodeURIComponent(value) { return keys.has(value) ? `<strong>${value}</strong>` : htmlSafe(value); },
2526
});
2627

2728
const { SessionNotFound } = errors;

example/support/html_safe.js

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
export default function htmlSafe(input) {
2+
if (typeof input === 'number' && Number.isFinite(input)) {
3+
return `${input}`;
4+
}
5+
6+
if (typeof input === 'string') {
7+
return input.replace(/&/g, '&amp;')
8+
.replace(/</g, '&lt;')
9+
.replace(/>/g, '&gt;')
10+
.replace(/"/g, '&quot;')
11+
.replace(/'/g, '&#39;');
12+
}
13+
14+
if (typeof input === 'boolean') {
15+
return input.toString();
16+
}
17+
18+
return '';
19+
}

lib/actions/interaction.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import * as querystring from 'node:querystring';
33
import { inspect } from 'node:util';
44

55
import * as attention from '../helpers/attention.js';
6+
import htmlSafe from '../helpers/html_safe.js';
67
import { urlencoded as parseBody } from '../shared/selective_body.js';
78
import * as views from '../views/index.js';
89
import instance from '../helpers/weak_cache.js';
@@ -16,7 +17,7 @@ const dbg = (obj) => querystring.stringify(Object.entries(obj).reduce((acc, [key
1617
acc[key] = inspect(value, { depth: null });
1718
return acc;
1819
}, {}), '<br/>', ': ', {
19-
encodeURIComponent(value) { return keys.has(value) ? `<strong>${value}</strong>` : value; },
20+
encodeURIComponent(value) { return keys.has(value) ? `<strong>${value}</strong>` : htmlSafe(value); },
2021
});
2122

2223
export default function devInteractions(provider) {

test/interaction/interaction.test.js

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,32 @@ describe('devInteractions', () => {
7878
handlesInteractionSessionErrors();
7979
});
8080

81+
context('escapes debug output HTML', () => {
82+
beforeEach(function () { return this.logout(); });
83+
beforeEach(function () {
84+
const auth = new this.AuthorizationRequest({
85+
response_type: 'code',
86+
scope: 'openid',
87+
state: '<img src=x onerror=alert(1)>',
88+
});
89+
90+
return this.agent.get('/auth')
91+
.query(auth)
92+
.then((response) => {
93+
this.url = response.headers.location;
94+
});
95+
});
96+
97+
it('HTML-escapes parameter values in the debug section', function () {
98+
return this.agent.get(this.url)
99+
.expect(200)
100+
.expect((response) => {
101+
expect(response.text).not.to.match(/<img src=x onerror=alert\(1\)>/);
102+
expect(response.text).to.contain('&lt;img src=x onerror=alert(1)&gt;');
103+
});
104+
});
105+
});
106+
81107
context('render interaction', () => {
82108
beforeEach(function () { return this.logout(); });
83109
beforeEach(function () { return this.login(); });

0 commit comments

Comments
 (0)