Skip to content

Commit

Permalink
Support allowed scheme with openUrl card action (#3226)
Browse files Browse the repository at this point in the history
* Support allowet pushd scheme with `openUrl` card action

* Update entry

* Fix test

* Fix tests

* Fix tests

* Apply suggestions from code review

Co-authored-by: Corina <[email protected]>

Co-authored-by: Corina <[email protected]>
  • Loading branch information
compulim and corinagum authored Jun 9, 2020
1 parent 7387c32 commit 484291a
Show file tree
Hide file tree
Showing 7 changed files with 286 additions and 4 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,16 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.

## [Unreleased]

### Breaking changes

- Affecting Adaptive Cards, legacy cards and suggested actions
- For `openUrl` card action, we are now whitelisting the URL scheme using the same whitelist from the default Markdown + sanitize engine, which includes `data`, `http`, `https`, `ftp`, `mailto`, `sip`, and `tel`
- To whitelist a different set of URL schemes, please implement the card action middleware to override this behavior

### Added

- Resolves [#3205](https://github.com/microsoft/BotFramework-WebChat/issues/3205). Added Direct Line App Service Extension protocol, by [@compulim](https://github.com/compulim) in PR [#3206](https://github.com/microsoft/BotFramework-WebChat/pull/3206)
- Resolves [#3225](https://github.com/microsoft/BotFramework-WebChat/issues/3225). Support allowed scheme with `openUrl` card action, by [@compulim](https://github.com/compulim) in PR [#3226](https://github.com/microsoft/BotFramework-WebChat/pull/3226)

### Fixed

Expand Down
115 changes: 115 additions & 0 deletions __tests__/html/cardAction.adaptiveCard.disallowedScheme.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
<!DOCTYPE html>
<html lang="en-US">
<head>
<script crossorigin="anonymous" src="/__dist__/testharness.js"></script>
<script crossorigin="anonymous" src="/__dist__/webchat-es5.js"></script>
</head>
<body>
<div id="webchat"></div>
<script type="text/babel" data-presets="env,stage-3,react">
const {
conditions,
createRunHookActivityMiddleware,
createStore,
elements,
expect,
getConsoleHistory,
host,
pageObjects,
timeouts,
token
} = window.WebChatTest;

function stringToArrayBuffer(value) {
// This assume the string is ASCII (0-127).

const { length } = value;
const byteArray = new Array(length);

for (let index = 0; index < length; index++) {
const charCode = value.charCodeAt(index);

if (charCode > 127) {
throw new Error('Only ASCII characters are supported.');
}

byteArray[index] = charCode;
}

return new Uint8Array(byteArray).buffer;
}

(async function() {
window.WebChat.renderWebChat(
{
activityMiddleware: createRunHookActivityMiddleware(),
directLine: window.WebChat.createDirectLine({ token: await token.fetchDirectLineToken() }),
store: createStore()
},
document.getElementById('webchat')
);

await pageObjects.wait(conditions.uiConnected(), timeouts.directLine);

const fileBlob = new Blob([
stringToArrayBuffer(
JSON.stringify(
{
contentType: 'application/vnd.microsoft.card.adaptive',
content: {
type: 'AdaptiveCard',
body: [
{
type: 'TextBlock',
size: 'Medium',
text: 'Tap on this Adaptive Card will open Bing.com.'
}
],
$schema: 'http://adaptivecards.io/schemas/adaptive-card.json',
version: '1.2',
selectAction: {
type: 'Action.OpenUrl',
url: 'javascript:alert(1)'
}
}
},
null,
2
)
)
]);

fileBlob.name = 'openurl-card.attachmentjson';

await pageObjects.runHook(({ useSendFiles }) => useSendFiles()([fileBlob]));
await pageObjects.wait(conditions.minNumActivitiesShown(2), timeouts.directLine);

const calls = [];

window.open = (url, windowName, windowFeatures) => calls.push([url, windowName, windowFeatures]);

const adaptiveCard = elements.activities()[1].querySelector('.ac-adaptiveCard');

adaptiveCard.click();

expect(calls).toHaveProperty('length', 0);

// Expect to show a warning.
await expect(getConsoleHistory()).toEqual(
expect.arrayContaining([
expect.objectContaining({
args: expect.arrayContaining([expect.stringContaining('disallowed scheme')]),
level: 'warn'
})
])
);

await host.done();
})().catch(async err => {
console.error(err);

await host.error(err);
});
</script>
</body>
</html>
7 changes: 7 additions & 0 deletions __tests__/html/cardAction.adaptiveCard.disallowedScheme.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/**
* @jest-environment ./__tests__/html/__jest__/WebChatEnvironment.js
*/

describe('"openUrl" action on Adaptive Card', () => {
test('with a disallowed scheme should not open', () => runHTMLTest('cardAction.adaptiveCard.disallowedScheme.html'));
});
106 changes: 106 additions & 0 deletions __tests__/html/cardAction.heroCard.disallowedScheme.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
<!DOCTYPE html>
<html lang="en-US">
<head>
<script crossorigin="anonymous" src="/__dist__/testharness.js"></script>
<script crossorigin="anonymous" src="/__dist__/webchat-es5.js"></script>
</head>
<body>
<div id="webchat"></div>
<script type="text/babel" data-presets="env,stage-3,react">
const {
conditions,
createRunHookActivityMiddleware,
createStore,
elements,
expect,
getConsoleHistory,
host,
pageObjects,
timeouts,
token
} = window.WebChatTest;

function stringToArrayBuffer(value) {
// This assume the string is ASCII (0-127).

const { length } = value;
const byteArray = new Array(length);

for (let index = 0; index < length; index++) {
const charCode = value.charCodeAt(index);

if (charCode > 127) {
throw new Error('Only ASCII characters are supported.');
}

byteArray[index] = charCode;
}

return new Uint8Array(byteArray).buffer;
}

(async function() {
window.WebChat.renderWebChat(
{
activityMiddleware: createRunHookActivityMiddleware(),
directLine: window.WebChat.createDirectLine({ token: await token.fetchDirectLineToken() }),
store: createStore()
},
document.getElementById('webchat')
);

await pageObjects.wait(conditions.uiConnected(), timeouts.directLine);

const fileBlob = new Blob([
stringToArrayBuffer(
JSON.stringify(
{
contentType: 'application/vnd.microsoft.card.hero',
content: {
tap: {
type: 'openUrl',
value: 'javascript:alert(1)'
},
title: 'Tap on this hero card will open Bing.com.'
}
},
null,
2
)
)
]);

fileBlob.name = 'openurl-card.attachmentjson';

await pageObjects.runHook(({ useSendFiles }) => useSendFiles()([fileBlob]));
await pageObjects.wait(conditions.minNumActivitiesShown(2), timeouts.directLine);

const calls = [];

window.open = (url, windowName, windowFeatures) => calls.push([url, windowName, windowFeatures]);

const adaptiveCard = elements.activities()[1].querySelector('.ac-adaptiveCard');

adaptiveCard.click();

expect(calls).toHaveProperty('length', 0);

// Expect to show a warning.
await expect(getConsoleHistory()).toEqual(
expect.arrayContaining([
expect.objectContaining({
args: expect.arrayContaining([expect.stringContaining('disallowed scheme')]),
level: 'warn'
})
])
);

await host.done();
})().catch(async err => {
console.error(err);

await host.error(err);
});
</script>
</body>
</html>
7 changes: 7 additions & 0 deletions __tests__/html/cardAction.heroCard.disallowedScheme.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/**
* @jest-environment ./__tests__/html/__jest__/WebChatEnvironment.js
*/

describe('"openUrl" action on hero card', () => {
test('with a disallowed scheme should not open', () => runHTMLTest('cardAction.heroCard.disallowedScheme.html'));
});
12 changes: 9 additions & 3 deletions __tests__/html/speechRecognition.simple.html
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,15 @@
}

try {
expect(recognized.map(({ text }) => text)).toEqual([
"Unknown command I don't know. Hello world you can say help to learn more."
]);
expect(
recognized.map(({ text }) =>
text
.replace(/[^\w']/giu, ' ')
.replace(new RegExp('\\s+', 'gu'), ' ')
.trim()
.toLowerCase()
)
).toEqual(["unknown command i don't know hello world you can say help to learn more"]);
} catch (err) {
const files = recognized.map(({ filename }) => `See diff for details: ${filename}`).join('\n');
const detailedError = new Error(err.message + '\n\n' + files);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,34 @@
import { sendMessage, sendMessageBack, sendPostBack } from 'botframework-webchat-core';

// This code is adopted from sanitize-html/naughtyScheme.
// sanitize-html is a dependency of Web Chat but the naughtScheme function is neither exposed nor reusable.
// https://github.com/apostrophecms/sanitize-html/blob/master/src/index.js#L526
function getScheme(href) {
// Browsers ignore character codes of 32 (space) and below in a surprising
// number of situations. Start reading here:
// https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Embedded_tab
// eslint-disable-next-line no-control-regex

href = href.replace(/[\x00-\x20]+/g, '');

// Clobber any comments in URLs, which the browser might
// interpret inside an XML data island, allowing
// a javascript: URL to be snuck through
href = href.replace(/<\!\-\-.*?\-\-\>/g, '');

// Case insensitive so we don't get faked out by JAVASCRIPT #1
const matches = href.match(/^([a-zA-Z]+)\:/);

if (!matches) {
// Protocol-relative URL or no scheme
return;
}

return matches[1].toLowerCase();
}

const ALLOWED_SCHEMES = ['data', 'http', 'https', 'ftp', 'mailto', 'sip', 'tel'];

export default function createDefaultCardActionMiddleware() {
return ({ dispatch }) => next => ({ cardAction, getSignInUrl }) => {
const { displayText, text, type, value } = cardAction;
Expand Down Expand Up @@ -31,7 +60,12 @@ export default function createDefaultCardActionMiddleware() {
case 'playAudio':
case 'playVideo':
case 'showImage':
window.open(value, '_blank', 'noopener noreferrer');
if (ALLOWED_SCHEMES.includes(getScheme(value))) {
window.open(value, '_blank', 'noopener noreferrer');
} else {
console.warn('botframework-webchat: Cannot open URL with disallowed schemes.', value);
}

break;

case 'signin': {
Expand Down

0 comments on commit 484291a

Please sign in to comment.