Skip to content

Commit

Permalink
Changes interpolation strategy to sanitize
Browse files Browse the repository at this point in the history
Adds ngSanitize so we can use HTML in translation strings, changes
angular-translate's sanitization strategy to `sanitize` instead of
`escape`.

My motivation here is to make the translation strings be non-ridiculous.
Without being able to inject un-escaped HTML into the translation, we
end up chopping up the translation keys to the point where they're not
comprehensible within lokalise, and doing weird gymnastics in the
angular template like `<span translate>key</span>: <strong
translate>value</strong>` because both are contained within the same
parent.

The cost here is that there's an [open bug on angular-translate][1] for
the `sanitize` strategy where unicode gets double-encoded. I tried `sce`
as a strategy, but it seemed open to XSS (ie. include
`<script>alert('xss')</script>` in a translation and you'll see the
alert. The `sanitize` strategy successfully killed this vector). The
cost here is that as much as possible we need to use the `<tag
translate>` form instead of the `{$ 'string' | translate $}` form.

For the most part, we're not translating arbitrary user input, but no
sense in leaving that door open for us to forget about down the road.

[1]: angular-translate/angular-translate#1101
  • Loading branch information
rgm committed Nov 3, 2017
1 parent c4604d7 commit dc3c38c
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 3 deletions.
3 changes: 2 additions & 1 deletion bower.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@
"angular-dragula": "^1.2.8",
"ng-focus-if": "^1.0.7",
"angular-translate": "^2.15.2",
"angular-translate-loader-static-files": "^2.15.2"
"angular-translate-loader-static-files": "^2.15.2",
"angular-sanitize": "^1.6.6"
},
"resolutions": {
"angular": "~1.6.1",
Expand Down
5 changes: 3 additions & 2 deletions seed/static/seed/js/seed.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ angular.module('BE.seed.vendor_dependencies', [
'focus-if',
'xeditable',
angularDragula(angular),
'pascalprecht.translate'
'pascalprecht.translate',
'ngSanitize'
]);
angular.module('BE.seed.controllers', [
'BE.seed.controller.about',
Expand Down Expand Up @@ -1109,7 +1110,7 @@ SEED_app.config(['$translateProvider', function ($translateProvider) {
'fr_*': 'fr_CA'
})
// see https://angular-translate.github.io/docs/#/guide/19_security
.useSanitizeValueStrategy('escape');
.useSanitizeValueStrategy('sanitize');

$translateProvider.determinePreferredLanguage();
moment.locale($translateProvider.preferredLanguage());
Expand Down
1 change: 1 addition & 0 deletions seed/templates/seed/base.html
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@
<script src="/static/vendors/bower_components/spin.js/spin.min.js"></script>
<script src="/static/vendors/bower_components/angular-translate/angular-translate.min.js"></script>
<script src="/static/vendors/bower_components/angular-translate-loader-static-files/angular-translate-loader-static-files.min.js"></script>
<script src="/static/vendors/bower_components/angular-sanitize/angular-sanitize.min.js"></script>


{% ifequal FILE_UPLOAD_DESTINATION 'S3' %}
Expand Down

0 comments on commit dc3c38c

Please sign in to comment.