Add Content Security Policy header, fixed obvious security flaws

unsafe-eval is still permitted due to use of underscore templates. Fixing that will be a larger undertaking, Changes: * switched templates to escaping injected content * similarly, switched json output to using jquery.text() instead of .html() * moved google analytics js to external file to allow unsafe-inline js to be disabled * also updated to latest SDK and added .env support to aid testing
personality-insights · Sep 9, 2016 · f46b8ab · f46b8ab
1 parent 9c55d4b
commit f46b8ab
Show file tree

Hide file tree

Showing 9 changed files with 117 additions and 61 deletions.
diff --git a/.env.example b/.env.example
@@ -0,0 +1,3 @@
+# see http://www.ibm.com/watson/developercloud/doc/getting_started/gs-credentials.shtml
+TONE_ANALYZER_USERNAME=<username>
+TONE_ANALYZER_PASSWORD=<password>
diff --git a/.eslintignore b/.eslintignore
@@ -1,2 +1,2 @@
 coverage
-public/js/vendors/prism.js
+public/js/vendors/
diff --git a/app.js b/app.js
@@ -16,20 +16,21 @@
 
 'use strict';
 
-var express    = require('express'),
-  app          = express(),
-  watson       = require('watson-developer-cloud');
+require('dotenv').load({silent: true});
+var express = require('express'),
+  app = express();
+var ToneAnalyzerV3 = require('watson-developer-cloud/tone-analyzer/v3');
 
 // Bootstrap application settings
 require('./config/express')(app);
 
 // Create the service wrapper
-var toneAnalyzer = watson.tone_analyzer({
-  url: 'https://gateway.watsonplatform.net/tone-analyzer/api/',
-  username: '<username>',
-  password: '<password>',
-  version_date: '2016-05-19',
-  version: 'v3'
+var toneAnalyzer = new ToneAnalyzerV3({
+  // If unspecified here, the TONE_ANALYZER_USERNAME and TONE_ANALYZER_PASSWORD environment properties will be checked
+  // After that, the SDK will fall back to the bluemix-provided VCAP_SERVICES environment property
+  // username: '<username>',
+  // password: '<password>',
+  version_date: '2016-05-19'
 });
 
 app.get('/', function(req, res) {

diff --git a/config/security.js b/config/security.js
@@ -25,8 +25,58 @@ var cookieParser = require('cookie-parser');
 module.exports = function(app) {
   app.enable('trust proxy');
 
-  // 1. helmet with defaults
-  app.use(helmet({ cacheControl: false }));
+  // 1. helmet with custom CSP header
+  var cspReportUrl = '/report-csp-violation';
+  app.use(helmet({
+    cacheControl: false,
+    contentSecurityPolicy: {
+      // Specify directives as normal.
+      directives: {
+        defaultSrc: ["'self'"], // default value for unspecified directives that end in -src
+        scriptSrc: [
+          "'self'",
+          "'unsafe-eval'", // underscore.js requires this for templates :(
+          'https://cdnjs.cloudflare.com/',
+          'https://ajax.googleapis.com/',
+          'www.google-analytics.com'
+        ], // jquery cdn, etc. try to avid "'unsafe-inline'" and "'unsafe-eval'"
+        styleSrc: ["'self'", "'unsafe-inline'"], // no inline css
+        imgSrc: ['*', 'data:'], // should be "'self'" and possibly 'data:' for most apps, but vr demo loads random user-supplied image urls, and apparently * doesn't include data: URIs
+        connectSrc: ["'self'", '*.watsonplatform.net'], // ajax domains
+        // fontSrc: ["'self'"], // cdn?
+        objectSrc: [], // embeds (e.g. flash)
+        // mediaSrc: ["'self'", '*.watsonplatform.net'], // allow watson TTS streams
+        childSrc: [], // child iframes
+        frameAncestors: [], // parent iframes
+        formAction: ["'self'"], // where can forms submit to
+        pluginTypes: [], // e.g. flash, pdf
+        // sandbox: ['allow-forms', 'allow-scripts', 'allow-same-origin'], // options: allow-forms allow-same-origin allow-scripts allow-top-navigation
+        reportUri: cspReportUrl
+      },
+
+      // Set to true if you only want browsers to report errors, not block them.
+      // You may also set this to a function(req, res) in order to decide dynamically
+      // whether to use reportOnly mode, e.g., to allow for a dynamic kill switch.
+      reportOnly: false,
+
+      // Set to true if you want to blindly set all headers: Content-Security-Policy,
+      // X-WebKit-CSP, and X-Content-Security-Policy.
+      setAllHeaders: false,
+
+      // Set to true if you want to disable CSP on Android where it can be buggy.
+      disableAndroid: false,
+
+      // Set to false if you want to completely disable any user-agent sniffing.
+      // This may make the headers less compatible but it will be much faster.
+      // This defaults to `true`.
+      browserSniff: true
+    }
+  }));
+  // endpoint to report CSP violations
+  app.post(cspReportUrl, function(req, res) {
+    console.log('Content Security Policy Violation:\n', req.body);
+    res.status(204).send(); // 204 = No Content
+  });
 
   // 2. rate limiting
   app.use('/api/', rateLimit({

diff --git a/package.json b/package.json
@@ -44,7 +44,7 @@
     "express-rate-limit": "^2.1.0",
     "express-secure-only": "^0.2.1",
     "helmet": "^2.1.1",
-    "watson-developer-cloud": "^1.12.4"
+    "watson-developer-cloud": "^2.2.0"
   },
   "devDependencies": {
     "babel-eslint": "^6.0.4",

diff --git a/public/js/components/App.js b/public/js/components/App.js
@@ -308,7 +308,6 @@ function App(documentTones, sentences, thresholds, selectedSample) { // eslint-d
     var map = function(item) {
       var result = item;
       result.className = _toneLevel(_selectedFilter, item.tone_categories[_searchIndex(_selectedTone)].tones[_searchIndex(_selectedFilter)].score, 'className_OT');
-      result.text = result.text.replace(/\r?\n/g, '<br />');
       return result;
     };
     return _originalSentences.map(map);

diff --git a/public/js/demo.js b/public/js/demo.js
@@ -315,7 +315,7 @@ function allReady(thresholds, sampleText) {
      */
     function updateJSONSentenceTones() {
       $sentenceJson.empty();
-      $sentenceJson.html(JSON.stringify({'sentences_tone': data.sentences_tone}, null, 2));
+      $sentenceJson.text(JSON.stringify({'sentences_tone': data.sentences_tone}, null, 2));
     }
 
     /**
@@ -324,7 +324,7 @@ function allReady(thresholds, sampleText) {
      */
     function updateJSONDocumentTones() {
       $summaryJsonCode.empty();
-      $summaryJsonCode.html(JSON.stringify({'document_tone': data.document_tone}, null, 2));
+      $summaryJsonCode.text(JSON.stringify({'document_tone': data.document_tone}, null, 2));
     }
 
     /**
@@ -403,7 +403,7 @@ function allReady(thresholds, sampleText) {
       message = 'You\'ve sent a lot of requests in a short amount of time. ' +
         'As the CPU cores cool off a bit, wait a few seonds before sending more requests.';
     }
-    $errorMessage.html(message);
+    $errorMessage.text(message);
     $input.show();
     $loading.hide();
     $output.hide();

diff --git a/public/js/vendors/google-analytics.js b/public/js/vendors/google-analytics.js
@@ -0,0 +1,7 @@
+(function(i,s,o,g,r,a,m){i['GoogleAnalyticsObject']=r;i[r]=i[r]||function(){
+        (i[r].q=i[r].q||[]).push(arguments)},i[r].l=1*new Date();a=s.createElement(o),
+    m=s.getElementsByTagName(o)[0];a.async=1;a.src=g;m.parentNode.insertBefore(a,m)
+})(window,document,'script','//www.google-analytics.com/analytics.js','ga');
+
+ga('create', '<$= ga $>', 'auto');
+ga('send', 'pageview');
diff --git a/views/index.ejs b/views/index.ejs
@@ -615,29 +615,29 @@
 
   <script type="text/html" id="emotionBarGraphTemplate">
     <% _.each(items, function(item, key, list) { %>
-      <div class="bar-graph--row summary-<%= className %>-graph--row">
-        <div class="bar-graph--label-container summary-<%= className %>-graph--label-container">
+      <div class="bar-graph--row summary-<%- className %>-graph--row">
+        <div class="bar-graph--label-container summary-<%- className %>-graph--label-container">
           <div class="bar-graph--label">
             <p class="base--p">
-              <%= item.label %>
+              <%- item.label %>
             </p>
             <% if (item.tooltip) { %>
               <div class="bar-graph--tooltip">
-                <%= item.tooltip %>
+                <%- item.tooltip %>
               </div>
               <% } %>
           </div>
         </div>
-        <div class="bar-graph--bar-container summary-<%= className %>-graph--bar-container">
+        <div class="bar-graph--bar-container summary-<%- className %>-graph--bar-container">
           <div class="bar-graph--bar">
-            <div class="bar-graph--bar-value summary-<%= className %>-graph--bar-value summary-<%= className %>-graph--bar-value_<%= item.label.replace(/\s+/g, '-') %>" style="width: <%= item.score %>%;"></div>
+            <div class="bar-graph--bar-value summary-<%- className %>-graph--bar-value summary-<%- className %>-graph--bar-value_<%- item.label.replace(/\s+/g, '-') %>" style="width: <%- item.score %>%;"></div>
           </div>
         </div>
         <div class="summary-emotion-graph--percentage-label">
-          <%= (item.score / 100).toFixed(2) %>
+          <%- (item.score / 100).toFixed(2) %>
           <br>
           <span class="summary-emotion-graph--percentage-label-likeliness">
-            <%= item.likeliness %>
+            <%- item.likeliness %>
           </span>
         </div>
       </div>
@@ -647,59 +647,63 @@
   <script type="text/html" id="barGraphTemplate">
     <% _.each(items, function(item, key, list) { %>
       <div class="bar-graph--row">
-        <div class="bar-graph--label-container summary-<%= className %>-graph--label-container">
+        <div class="bar-graph--label-container summary-<%- className %>-graph--label-container">
           <div class="bar-graph--label">
             <p class="base--p">
-              <%= item.label %>
+              <%- item.label %>
             </p>
             <% if (item.tooltip) { %>
               <div class="bar-graph--tooltip">
-                <%= item.tooltip %>
+                <%- item.tooltip %>
               </div>
               <% } %>
           </div>
         </div>
-        <div class="bar-graph--bar-container summary-<%= className %>-graph--bar-container">
+        <div class="bar-graph--bar-container summary-<%- className %>-graph--bar-container">
           <div class="bar-graph--bar">
-            <div class="bar-graph--bar-value summary-<%= className %>-graph--bar-value summary-<%= className %>-graph--bar-value_<%= item.label.replace(/\s+/g, '-') %>" style="width: <%= item.score %>%;"></div>
+            <div class="bar-graph--bar-value summary-<%- className %>-graph--bar-value summary-<%- className %>-graph--bar-value_<%- item.label.replace(/\s+/g, '-') %>" style="width: <%- item.score %>%;"></div>
           </div>
         </div>
         <div class="bar-graph--percentage-label">
-          <%= (item.score / 100).toFixed(2) %>
+          <%- (item.score / 100).toFixed(2) %>
         </div>
       </div>
       <% }); %>
   </script>
 
   <script type="text/html" id="verticalBarGraphTemplate">
     <% _.each(items, function(item, key, list) { %>
-      <div class="summary-<%= className %>-graph--column">
-        <div class="summary-<%= className %>-graph--bar">
-          <div class="summary-<%= className %>-graph--bar-value" style="height: <%= item.score %>%;"></div>
-          <div class="summary-<%= className %>-graph--bar-tooltip summary-<%= className %>-graph--bar-tooltip_<%= item.label.replace(/\s+/g, '-') %> bar-graph--bar-tooltip" style="margin-bottom: <%= item.score %>%;">
-            <%= (item.score / 100).toFixed(2) %>
+      <div class="summary-<%- className %>-graph--column">
+        <div class="summary-<%- className %>-graph--bar">
+          <div class="summary-<%- className %>-graph--bar-value" style="height: <%- item.score %>%;"></div>
+          <div class="summary-<%- className %>-graph--bar-tooltip summary-<%- className %>-graph--bar-tooltip_<%- item.label.replace(/\s+/g, '-') %> bar-graph--bar-tooltip" style="margin-bottom: <%- item.score %>%;">
+            <%- (item.score / 100).toFixed(2) %>
           </div>
         </div>
-        <div class="summary-<%= className %>-graph--label">
-          <%= item.label %>
+        <div class="summary-<%- className %>-graph--label">
+          <%- item.label %>
         </div>
       </div>
       <% }); %>
   </script>
 
   <script type="text/html" id="filtersTemplate">
     <% _.each(items, function(item, key, list) { %>
-      <input class="filters--radio base--radio" type="radio" id="filter-<%= item.label.replace(/\s+/g, '-') %>" data-id="<%= item.label %>" name="tone-filters" value="<%= item.label.replace(/\s+/g, '-') %>">
-      <label class="filters--label base--block-label" for="filter-<%= item.label.replace(/\s+/g, '-') %>">
-        <%= item.label %>
+      <input class="filters--radio base--radio" type="radio" id="filter-<%- item.label.replace(/\s+/g, '-') %>" data-id="<%- item.label %>" name="tone-filters" value="<%- item.label.replace(/\s+/g, '-') %>">
+      <label class="filters--label base--block-label" for="filter-<%- item.label.replace(/\s+/g, '-') %>">
+        <%- item.label %>
       </label>
       <% }); %>
   </script>
 
   <script type="text/html" id="originalTextTemplate">
     <% _.each(items, function(item, key, list) { %>
-      <div class="base--p original-text--sentence-container" data-index="<%= key %>">
-        <span class="original-text--sentence <%= item.className %>"><%= item.text %></span>
+      <div class="base--p original-text--sentence-container" data-index="<%- key %>">
+        <span class="original-text--sentence <%- item.className %>">
+          <% item.text.split(/\r?\n/g).forEach(function(line, index, arr) { %>
+            <%- line %>
+            <% if (index +1 != arr.length) { %><br /><% } %>
+          <% }) %></span>
       </div>
       <% }); %>
   </script>
@@ -708,23 +712,23 @@
     <tbody class="base--tbody">
       <% _.each(items, function(item, key, list) { %>
         <tr class="base--tr sentence-rank--tr">
-          <td class="base--td sentence-rank--score <%= item.className %>"><b><%= item.score %></b></td>
+          <td class="base--td sentence-rank--score <%- item.className %>"><b><%- item.score %></b></td>
           <td class="base--td sentence-rank--sentence">
-            <%= item.text %>
+            <%- item.text %>
           </td>
         </tr>
         <% }); %>
     </tbody>
   </script>
 
   <script type="text/html" id="originalTextTooltipTemplate">
-    <div class="original-text--tooltip <%= isSocialTone == 'Social Tone' ? 'original-text--tooltip_social' : '' %>">
+    <div class="original-text--tooltip <%- isSocialTone == 'Social Tone' ? 'original-text--tooltip_social' : '' %>">
       <ul class="original-text--tooltip-ul">
         <% _.each(items, function(item, key, list) { %>
-          <li class="original-text--tooltip-li <%= item.className %>">
-            <div class="original-text--tooltip-score"><b><%= item.score_percentage %></b></div>
+          <li class="original-text--tooltip-li <%- item.className %>">
+            <div class="original-text--tooltip-score"><b><%- item.score_percentage %></b></div>
             <div class="original-text--tooltip-label">
-              <%= item.tone_name %>
+              <%- item.tone_name %>
             </div>
           </li>
           <% }); %>
@@ -734,9 +738,9 @@
 
   <script type="text/html" id="originalTextLegendTemplate">
     <div class="original-text--legend-bar">
-      <div class="original-text--legend-block original-text--legend-block_low original-text--legend-block_<%= className %>-low"></div>
-      <div class="original-text--legend-block original-text--legend-block_med original-text--legend-block_<%= className %>-medium"></div>
-      <div class="original-text--legend-block original-text--legend-block_high original-text--legend-block_<%= className %>-high"></div>
+      <div class="original-text--legend-block original-text--legend-block_low original-text--legend-block_<%- className %>-low"></div>
+      <div class="original-text--legend-block original-text--legend-block_med original-text--legend-block_<%- className %>-medium"></div>
+      <div class="original-text--legend-block original-text--legend-block_high original-text--legend-block_<%- className %>-high"></div>
       <div class="original-text--legend-label original-text--legend-label_low">None</div>
       <div class="original-text--legend-label original-text--legend-label_high">Strong</div>
     </div>
@@ -749,15 +753,7 @@
   <script type="text/javascript" src="js/components/helpers.js"></script>
   <script type="text/javascript" src="js/components/App.js"></script>
   <script type="text/javascript" src="js/demo.js"></script>
-  <script type="text/javascript">
-    (function(i,s,o,g,r,a,m){i['GoogleAnalyticsObject']=r;i[r]=i[r]||function(){
-    (i[r].q=i[r].q||[]).push(arguments)},i[r].l=1*new Date();a=s.createElement(o),
-    m=s.getElementsByTagName(o)[0];a.async=1;a.src=g;m.parentNode.insertBefore(a,m)
-    })(window,document,'script','//www.google-analytics.com/analytics.js','ga');
-
-    ga('create', '<$= ga $>', 'auto');
-    ga('send', 'pageview');
-  </script>
+  <script type="text/javascript" src="js/vendors/google-analytics.js"></script>
 </body>
 
 </html>