Skip to content

Commit

Permalink
Warnings for dangerous files
Browse files Browse the repository at this point in the history
  • Loading branch information
scottnonnenberg-signal committed Oct 9, 2018
1 parent 3b8f934 commit ca61c9c
Show file tree
Hide file tree
Showing 15 changed files with 232 additions and 13 deletions.
5 changes: 5 additions & 0 deletions _locales/en/messages.json
Expand Up @@ -546,6 +546,11 @@
"message": "Unsupported file type",
"description": "Displayed for outgoing unsupported attachment"
},
"dangerousFileType": {
"message": "Attachment type not allowed for security reasons",
"description":
"Shown in toast when user attempts to send .exe file, for example"
},
"fileSizeWarning": {
"message": "Sorry, the selected file exceeds message size restrictions."
},
Expand Down
22 changes: 22 additions & 0 deletions images/error-filled.svg
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 2 additions & 1 deletion js/models/messages.js
Expand Up @@ -439,10 +439,11 @@
message: this,
}),

onDownload: () =>
onDownload: isDangerous =>
this.trigger('download', {
attachment: firstAttachment,
message: this,
isDangerous,
}),
};
},
Expand Down
23 changes: 23 additions & 0 deletions js/modules/types/attachment.js
Expand Up @@ -108,6 +108,29 @@ exports._replaceUnicodeOrderOverridesSync = attachment => {
exports.replaceUnicodeOrderOverrides = async attachment =>
exports._replaceUnicodeOrderOverridesSync(attachment);

// \u202A-\u202E is LRE, RLE, PDF, LRO, RLO
// \u2066-\u2069 is LRI, RLI, FSI, PDI
// \u200E is LRM
// \u200F is RLM
// \u061C is ALM
const V2_UNWANTED_UNICODE = /[\u202A-\u202E\u2066-\u2069\u200E\u200F\u061C]/g;

exports.replaceUnicodeV2 = async attachment => {
if (!is.string(attachment.fileName)) {
return attachment;
}

const fileName = attachment.fileName.replace(
V2_UNWANTED_UNICODE,
UNICODE_REPLACEMENT_CHARACTER
);

return {
...attachment,
fileName,
};
};

exports.removeSchemaVersion = ({ attachment, logger }) => {
if (!exports.isValid(attachment)) {
logger.error(
Expand Down
9 changes: 9 additions & 0 deletions js/modules/types/message.js
Expand Up @@ -44,6 +44,9 @@ const PRIVATE = 'private';
// Version 8
// - Attachments: Capture video/image dimensions and thumbnails, as well as a
// full-size screenshot for video.
// Version 9
// - Attachments: Expand the set of unicode characters we filter out of
// attachment filenames

const INITIAL_SCHEMA_VERSION = 0;

Expand Down Expand Up @@ -270,6 +273,11 @@ const toVersion8 = exports._withSchemaVersion({
upgrade: exports._mapAttachments(Attachment.captureDimensionsAndScreenshot),
});

const toVersion9 = exports._withSchemaVersion({
schemaVersion: 9,
upgrade: exports._mapAttachments(Attachment.replaceUnicodeV2),
});

const VERSIONS = [
toVersion0,
toVersion1,
Expand All @@ -280,6 +288,7 @@ const VERSIONS = [
toVersion6,
toVersion7,
toVersion8,
toVersion9,
];
exports.CURRENT_SCHEMA_VERSION = VERSIONS.length - 1;

Expand Down
9 changes: 8 additions & 1 deletion js/views/conversation_view.js
Expand Up @@ -1057,7 +1057,14 @@
}
},

downloadAttachment({ attachment, message }) {
downloadAttachment({ attachment, message, isDangerous }) {
if (isDangerous) {
const toast = new Whisper.DangerousFileTypeToast();
toast.$el.appendTo(this.$el);
toast.render();
return;
}

Signal.Types.Attachment.save({
attachment,
document,
Expand Down
15 changes: 14 additions & 1 deletion js/views/file_input_view.js
Expand Up @@ -34,6 +34,10 @@
template: i18n('unsupportedFileType'),
});

Whisper.DangerousFileTypeToast = Whisper.ToastView.extend({
template: i18n('dangerousFileType'),
});

Whisper.FileInputView = Backbone.View.extend({
tagName: 'span',
className: 'file-input',
Expand Down Expand Up @@ -178,6 +182,14 @@
if (!file) {
return;
}
const { name } = file;
if (window.Signal.Util.isFileDangerous(name)) {
const toast = new Whisper.DangerousFileTypeToast();
toast.$el.insertAfter(this.$el);
toast.render();

return;
}

const contentType = file.type;

Expand Down Expand Up @@ -297,9 +309,10 @@

getFile(rawFile) {
const file = rawFile || this.file || this.$input.prop('files')[0];
if (file === undefined) {
if (!file) {
return Promise.resolve();
}

const attachmentFlags = this.isVoiceNote
? textsecure.protobuf.AttachmentPointer.Flags.VOICE_MESSAGE
: null;
Expand Down
2 changes: 1 addition & 1 deletion stylesheets/_conversation.scss
Expand Up @@ -316,7 +316,7 @@
line-height: 18px;
letter-spacing: 0;

background-color: $color-light-60;
background-color: $color-gray-75;
color: $color-white;
box-shadow: 0 4px 16px 0 rgba(0, 0, 0, 0.12), 0 0 0 0.5px rgba(0, 0, 0, 0.08);
}
Expand Down
24 changes: 24 additions & 0 deletions stylesheets/_modules.scss
Expand Up @@ -345,6 +345,10 @@
padding-top: 4px;
}

.module-message__generic-attachment__icon-container {
position: relative;
}

.module-message__generic-attachment__icon {
background: url('../images/file-gradient.svg') no-repeat center;
height: 44px;
Expand All @@ -359,6 +363,26 @@
align-items: center;
}

.module-message__generic-attachment__icon-dangerous-container {
position: absolute;

top: -1px;
right: -4px;

height: 16px;
width: 16px;

border-radius: 50%;
background-color: $color-white;
}

.module-message__generic-attachment__icon-dangerous {
height: 16px;
width: 16px;

@include color-svg('../images/error-filled.svg', $color-core-red);
}

.module-message__generic-attachment__icon__extension {
font-size: 10px;
line-height: 13px;
Expand Down
2 changes: 1 addition & 1 deletion stylesheets/_theme_dark.scss
Expand Up @@ -62,7 +62,7 @@ body.dark-theme {
}

.toast {
background-color: $color-light-60;
background-color: $color-gray-45;
color: $color-white;
box-shadow: 0 4px 16px 0 rgba(0, 0, 0, 0.12),
0 0 0 0.5px rgba(0, 0, 0, 0.08);
Expand Down
44 changes: 44 additions & 0 deletions test/modules/types/attachment_test.js
Expand Up @@ -83,6 +83,50 @@ describe('Attachment', () => {
);
});

describe('replaceUnicodeV2', () => {
it('should remove all bad characters', async () => {
const input = {
size: 1111,
fileName:
'file\u202A\u202B\u202C\u202D\u202E\u2066\u2067\u2068\u2069\u200E\u200F\u061C.jpeg',
};
const expected = {
fileName:
'file\uFFFD\uFFFD\uFFFD\uFFFD\uFFFD\uFFFD\uFFFD\uFFFD\uFFFD\uFFFD\uFFFD\uFFFD.jpeg',
size: 1111,
};

const actual = await Attachment.replaceUnicodeV2(input);
assert.deepEqual(actual, expected);
});

it('should should leave normal filename alone', async () => {
const input = {
fileName: 'normal.jpeg',
size: 1111,
};
const expected = {
fileName: 'normal.jpeg',
size: 1111,
};

const actual = await Attachment.replaceUnicodeV2(input);
assert.deepEqual(actual, expected);
});

it('should handle missing fileName', async () => {
const input = {
size: 1111,
};
const expected = {
size: 1111,
};

const actual = await Attachment.replaceUnicodeV2(input);
assert.deepEqual(actual, expected);
});
});

describe('removeSchemaVersion', () => {
it('should remove existing schema version', () => {
const input = {
Expand Down
42 changes: 42 additions & 0 deletions ts/components/conversation/Message.md
Expand Up @@ -1922,6 +1922,48 @@ Voice notes are not shown any differently from audio attachments.
</util.ConversationContext>
```
#### Dangerous file type
```jsx
<util.ConversationContext theme={util.theme}>
<li>
<Message
conversationColor="green"
direction="incoming"
i18n={util.i18n}
timestamp={Date.now()}
attachment={{
url: util.txtObjectUrl,
contentType: 'text/plain',
fileName: 'blah.exe',
fileSize: '3.05 KB',
}}
onClickAttachment={isDangerous =>
console.log('onClickAttachment - isDangerous:', isDangerous)
}
/>
</li>
<li>
<Message
conversationColor="green"
direction="outgoing"
i18n={util.i18n}
timestamp={Date.now()}
status="sent"
attachment={{
url: util.txtObjectUrl,
contentType: 'text/plain',
fileName: 'blah.exe',
fileSize: '3.05 KB',
}}
onClickAttachment={isDangerous =>
console.log('onClickAttachment - isDangerous:', isDangerous)
}
/>
</li>
</util.ConversationContext>
```
### In a group conversation
Note that the author avatar goes away if `collapseMetadata` is set.
Expand Down
30 changes: 23 additions & 7 deletions ts/components/conversation/Message.tsx
Expand Up @@ -14,6 +14,7 @@ import { ContactName } from './ContactName';
import { Quote, QuotedAttachment } from './Quote';
import { EmbeddedContact } from './EmbeddedContact';

import { isFileDangerous } from '../../util/isFileDangerous';
import { Contact } from '../../types/Contact';
import { Color, Localizer } from '../../types/Util';
import { ContextMenu, ContextMenuTrigger, MenuItem } from 'react-contextmenu';
Expand Down Expand Up @@ -87,7 +88,7 @@ export interface Props {
onClickAttachment?: () => void;
onReply?: () => void;
onRetrySend?: () => void;
onDownload?: () => void;
onDownload?: (isDangerous: boolean) => void;
onDelete?: () => void;
onShowDetail: () => void;
}
Expand Down Expand Up @@ -363,7 +364,7 @@ export class Message extends React.Component<Props, State> {
);
}

// tslint:disable-next-line max-func-body-length cyclomatic-complexity
// tslint:disable-next-line max-func-body-length cyclomatic-complexity jsx-no-lambda react-this-binding-issue
public renderAttachment() {
const {
i18n,
Expand Down Expand Up @@ -503,6 +504,7 @@ export class Message extends React.Component<Props, State> {
} else {
const { fileName, fileSize, contentType } = attachment;
const extension = getExtension({ contentType, fileName });
const isDangerous = isFileDangerous(fileName);

return (
<div
Expand All @@ -516,10 +518,17 @@ export class Message extends React.Component<Props, State> {
: null
)}
>
<div className="module-message__generic-attachment__icon">
{extension ? (
<div className="module-message__generic-attachment__icon__extension">
{extension}
<div className="module-message__generic-attachment__icon-container">
<div className="module-message__generic-attachment__icon">
{extension ? (
<div className="module-message__generic-attachment__icon__extension">
{extension}
</div>
) : null}
</div>
{isDangerous ? (
<div className="module-message__generic-attachment__icon-dangerous-container">
<div className="module-message__generic-attachment__icon-dangerous" />
</div>
) : null}
</div>
Expand Down Expand Up @@ -734,9 +743,16 @@ export class Message extends React.Component<Props, State> {
return null;
}

const fileName = attachment && attachment.fileName;
const isDangerous = isFileDangerous(fileName || '');

const downloadButton = attachment ? (
<div
onClick={onDownload}
onClick={() => {
if (onDownload) {
onDownload(isDangerous);
}
}}
role="button"
className={classNames(
'module-message__buttons__download',
Expand Down

0 comments on commit ca61c9c

Please sign in to comment.