CVE-2019-1010200: Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection') (CWE-78)
value: Voice Builder Prior to commit c145d4604df67e6fc625992412eef0bf9a85e26b and f6660e6d8f0d1d931359d591dbdec580fef36d36 is affected by: CWE-78: Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection'). The impact is: Remote code execution with the same privileges as the servers. The component is: Two web servers in the projects expose three vulnerable endpoints that can be accessed remotely. The endpoints are defined at: - /tts: https://github.com/google/voice-builder/blob/3a449a3e8d5100ff323161c89b897f6d5ccdb6f9/merlin_model_server/api.js#L34 - /alignment: https://github.com/google/voice-builder/blob/3a449a3e8d5100ff323161c89b897f6d5ccdb6f9/festival_model_server/api.js#L28 - /tts: https://github.com/google/voice-builder/blob/3a449a3e8d5100ff323161c89b897f6d5ccdb6f9/festival_model_server/api.js#L65. The attack vector is: Attacker sends a GET request to the vulnerable endpoint with a specially formatted query parameter. The fixed version is: After commit f6660e6d8f0d1d931359d591dbdec580fef36d36.
CWE description: The product constructs all or part of an OS command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended OS command when it is sent to a downstream component.
CVSS v3 base score: 9.8
Published: 2019-07-23T18:15Z
Repository: https://github.com/google/voice-builder
Language: JavaScript
Files changed: festival_model_server/api.js, festival_model_server/settings.js, festival_model_server/tests/UtilsTests.js, festival_model_server/utils.js
Commit date: 2018-09-25 10:58:45 +0800
Commit message: Added function to replace characters with spaces and use it to sanitize text

Change-Id: I45ebf2da031f5526c202642cd8946b7202a44e53

Diff: diff --git a/festival_model_server/api.js b/festival_model_server/api.js
index 0a40269..6543933 100644
--- a/festival_model_server/api.js
+++ b/festival_model_server/api.js
@@ -26,73 +26,78 @@ router.get('/healthz', (_req, res) => res.json('OK'));
 
 /** GET alignment information of the synthesized text. */
 router.get('/alignment', (req, res) => {
   const { text } = req.query;
   // TODO(twattanavekin): Test Unicode text.
-  const escapedText = JSON.stringify(text).slice(1, -1);
+  const sanitizedText = utils.replaceCharactersWithSpaces(text);
 
   const { festvoxFile, voiceName, localFolder } = utils.getVoiceSettings();
 
   // Example command:
+  // read txt; TEXT=${txt};
   // ${FESTIVALDIR}/bin/festival
   // -b festvox/goog_vb_unison_cg.scm  -b "(voice_goog_vb_unison_cg)"
   // -b "(Parameter.set 'Audio_Method 'Audio_Command)"
   // -b "(Parameter.set 'Audio_Command \"\")"
-  // -b "(utt.relation.print (SayText \" hello \") 'Segment)"
+  // -b "(utt.relation.print (SayText \" ${TEXT} \") 'Segment)";
 
-  const cmd = `${FESTIVAL_BIN} -b ${festvoxFile}  -b "(${voiceName})" \
+  const cmd = `read txt; TEXT=\$\{txt\}; ${FESTIVAL_BIN} -b ${festvoxFile}  -b "(${voiceName})" \
   -b "(Parameter.set 'Audio_Method 'Audio_Command)" \
   -b "(Parameter.set 'Audio_Command \\"\\")" \
-  -b "(utt.relation.print (SayText \\" ${escapedText} \\") 'Segment)"`;
+  -b "(utt.relation.print (SayText \\" \$\{TEXT\} \\") 'Segment)";`;
 
   const options = {
     cwd: localFolder,
     timeout: 10000,
     encoding: 'buffer',
   };
 
   console.log(`Running command - \n${cmd}`);
 
-  exec(cmd, options, (err, stdout, stderr) => {
+  const child = exec(cmd, options, (err, stdout, stderr) => {
     const errMsg = utils.getExecErrorMessage(err, stderr);
     if (errMsg) {
       return res.status(500).send(errMsg);
     }
     return res.status(200).send(utils.getAlignmentData(stdout.toString()));
   });
+  child.stdin.write(sanitizedText);
+  child.stdin.end();
 });
 
 /** GET synthesize voice based on a voice model */
 router.get('/tts', (req, res) => {
   const { text, type } = req.query;
   // TODO(twattanavekin): Test Unicode text.
-  const escapedText = JSON.stringify(text).slice(1, -1);
+  const sanitizedText = utils.replaceCharactersWithSpaces(text);
+  console.log(`Synthesizing ${sanitizedText}`);
 
-  console.log(`Synthesize "${text}"`);
   const { festvoxFile, voiceName, localFolder } = utils.getVoiceSettings();
 
   // Example command:
-  // echo "Hello world" | festival/bin/text2wave \
+  // festival/bin/text2wave \
   //  -eval festvox/goog_vb_unison_cg.scm \
   //  -eval "(voice_vb_unison_cg)"
-  const cmd = `echo "${escapedText}" \
-              | ${TEXT_TO_WAV_BIN} -eval ${festvoxFile} -eval "(${voiceName})"`;
+  const cmd = `${TEXT_TO_WAV_BIN} -eval ${festvoxFile} -eval "(${voiceName})"`;
   console.log(`Running command - \n${cmd}`);
 
   const options = {
     cwd: localFolder,
     timeout: 10000,
     encoding: 'buffer',
   };
-  exec(cmd, options, (err, stdout, stderr) => {
+
+  const child = exec(cmd, options, (err, stdout, stderr) => {
     const errMsg = utils.getExecErrorMessage(err, stderr);
     if (errMsg) {
       return res.status(500).send(errMsg);
     }
     res.status(200);
     res.set('Content-Type', 'audio/wav');
     const data = type === 'base64' ? stdout.toString('base64') : stdout;
     return res.send(data);
   });
+  child.stdin.write(sanitizedText);
+  child.stdin.end();
 });
 
 module.exports = router;
diff --git a/festival_model_server/settings.js b/festival_model_server/settings.js
new file mode 100644
index 0000000..deaee98
--- /dev/null
+++ b/festival_model_server/settings.js
@@ -0,0 +1,24 @@
+// Copyright 2018 Google Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+/**
+ * Module containing settings for merlin server.
+ */
+const settings = {
+  // Festival synthesis pipeline might not handle these characters as expected
+  // and also they are typically not needed during tts synthesis.
+  // Therefore we replace them with spaces.
+  CHARACTERS_TO_REPLACE_WITH_SPACES: '!"#$%&\'()*+,-./:;<=>?@[\\]^_`{|}~',
+};
+module.exports = settings;
diff --git a/festival_model_server/tests/UtilsTests.js b/festival_model_server/tests/UtilsTests.js
index dd0fac1..3e9c3aa 100644
--- a/festival_model_server/tests/UtilsTests.js
+++ b/festival_model_server/tests/UtilsTests.js
@@ -12,10 +12,38 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
 const utils = require('../utils');
 
+describe('Testing utils replaceCharactersWithSpaces method', () => {
+  it('The function should return characters replaced with spaces and trim', () => {
+    const testData = [
+      ['`<text>`', 'text'],
+      ['`text`', 'text'],
+      ['$(text)', 'text'],
+      ['"\$(text\)', 'text'],
+      ['\"\$(text\)', 'text'],
+      ['${{{text}', 'text'],
+      ['\$\{text\}', 'text'],
+      ['\${text"}', 'text'],
+      ['"${{text"}', 'text'],
+      ['"${text"}', 'text'],
+      ["\"ශ්රී ලංකාව'\"", 'ශ්රී ලංකාව'],
+      ['"$text"', 'text'],
+      ['$text', 'text'],
+      [",./;'[]*\-=", ''],
+      ['echo `<text>`', 'echo   text'],
+      ['echo $("text")', 'echo    text'],
+      ['echo \$\{text\}', 'echo   text'],
+    ];
+
+    testData.forEach(function(entry) {
+      expect(utils.replaceCharactersWithSpaces(entry[0])).toEqual(entry[1]);
+    });
+  });
+});
+
 describe('Testing utils getAlignmentData method', () => {
   const generateTestData = () => {
     let testContent = '()\n'; // Alignment data starts with this.
     const expectedContent = [];
     for (let i = 0; i < 20; i += 1) {
diff --git a/festival_model_server/utils.js b/festival_model_server/utils.js
index 4331e35..262781c 100644
--- a/festival_model_server/utils.js
+++ b/festival_model_server/utils.js
@@ -10,10 +10,12 @@
 // distributed under the License is distributed on an "AS IS" BASIS,
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+const settings = require('./settings');
+
 /**
  * Module containing util methods.
  */
 const { MODEL_LOCAL_PATH, UNISON_NAME } = require('./constants');
 
@@ -85,10 +87,22 @@ const getVoiceSettings = () => ({
   festvoxFile: `festvox/${UNISON_NAME}.scm`,
   voiceName: `voice_${UNISON_NAME}`,
   localFolder: MODEL_LOCAL_PATH,
 });
 
+/**
+ * Returns the original string with characters
+ * specified in settings replaced with spaces.
+ * @text {String} text to be replaced.
+ */
+const replaceCharactersWithSpaces = (text) => {
+  const re = new RegExp(`[${settings.CHARACTERS_TO_REPLACE_WITH_SPACES}]`, 'g');
+  const newText = text.replace(re, ' ').trim();
+  return newText;
+};
+
 module.exports = {
   getAlignmentData,
   getExecErrorMessage,
   getVoiceSettings,
+  replaceCharactersWithSpaces,
 };

Vulnerable code:   const escapedText = JSON.stringify(text).slice(1, -1);
  // -b "(utt.relation.print (SayText \" hello \") 'Segment)"
  const cmd = `${FESTIVAL_BIN} -b ${festvoxFile}  -b "(${voiceName})" \
  -b "(utt.relation.print (SayText \\" ${escapedText} \\") 'Segment)"`;
  exec(cmd, options, (err, stdout, stderr) => {
  const escapedText = JSON.stringify(text).slice(1, -1);
  console.log(`Synthesize "${text}"`);
  // echo "Hello world" | festival/bin/text2wave \
  const cmd = `echo "${escapedText}" \
              | ${TEXT_TO_WAV_BIN} -eval ${festvoxFile} -eval "(${voiceName})"`;
  exec(cmd, options, (err, stdout, stderr) => {
Fixed code:   const sanitizedText = utils.replaceCharactersWithSpaces(text);
  // read txt; TEXT=${txt};
  // -b "(utt.relation.print (SayText \" ${TEXT} \") 'Segment)";
  const cmd = `read txt; TEXT=\$\{txt\}; ${FESTIVAL_BIN} -b ${festvoxFile}  -b "(${voiceName})" \
  -b "(utt.relation.print (SayText \\" \$\{TEXT\} \\") 'Segment)";`;
  const child = exec(cmd, options, (err, stdout, stderr) => {
  child.stdin.write(sanitizedText);
  child.stdin.end();
  const sanitizedText = utils.replaceCharactersWithSpaces(text);
  console.log(`Synthesizing ${sanitizedText}`);
  // festival/bin/text2wave \
  const cmd = `${TEXT_TO_WAV_BIN} -eval ${festvoxFile} -eval "(${voiceName})"`;

  const child = exec(cmd, options, (err, stdout, stderr) => {
  child.stdin.write(sanitizedText);
  child.stdin.end();
// Copyright 2018 Google Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
//     https://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

/**
 * Module containing settings for merlin server.
 */
const settings = {
  // Festival synthesis pipeline might not handle these characters as expected
  // and also they are typically not needed during tts synthesis.
  // Therefore we replace them with spaces.
  CHARACTERS_TO_REPLACE_WITH_SPACES: '!"#$%&\'()*+,-./:;<=>?@[\\]^_`{|}~',
};
module.exports = settings;
describe('Testing utils replaceCharactersWithSpaces method', () => {
  it('The function should return characters replaced with spaces and trim', () => {
    const testData = [
      ['`<text>`', 'text'],
      ['`text`', 'text'],
      ['$(text)', 'text'],
      ['"\$(text\)', 'text'],
      ['\"\$(text\)', 'text'],
      ['${{{text}', 'text'],
      ['\$\{text\}', 'text'],
      ['\${text"}', 'text'],
      ['"${{text"}', 'text'],
      ['"${text"}', 'text'],
      ["\"ශ්රී ලංකාව'\"", 'ශ්රී ලංකාව'],
      ['"$text"', 'text'],
      ['$text', 'text'],
      [",./;'[]*\-=", ''],
      ['echo `<text>`', 'echo   text'],
      ['echo $("text")', 'echo    text'],
      ['echo \$\{text\}', 'echo   text'],
    ];

    testData.forEach(function(entry) {
      expect(utils.replaceCharactersWithSpaces(entry[0])).toEqual(entry[1]);
    });
  });
});

const settings = require('./settings');

/**
 * Returns the original string with characters
 * specified in settings replaced with spaces.
 * @text {String} text to be replaced.
 */
const replaceCharactersWithSpaces = (text) => {
  const re = new RegExp(`[${settings.CHARACTERS_TO_REPLACE_WITH_SPACES}]`, 'g');
  const newText = text.replace(re, ' ').trim();
  return newText;
};

  replaceCharactersWithSpaces,