From 1260eea690e4fa5420e58c9de9f88cc29cb242db Mon Sep 17 00:00:00 2001
From: undergroundwires <undergroundwires@users.noreply.github.com>
Date: Sun, 21 Feb 2021 12:34:33 +0100
Subject: [PATCH] escape printed characters to prevent command injection #45

---
 .../Code/Generation/Languages/BatchBuilder.ts |  8 +++-
 .../Code/Generation/Languages/ShellBuilder.ts |  7 +++-
 .../Generation/Languages/BatchBuilder.spec.ts | 42 ++++++++++++++-----
 .../Generation/Languages/ShellBuilder.spec.ts | 37 +++++++++++-----
 4 files changed, 72 insertions(+), 22 deletions(-)

diff --git a/src/application/Context/State/Code/Generation/Languages/BatchBuilder.ts b/src/application/Context/State/Code/Generation/Languages/BatchBuilder.ts
index 7a3668ef..bf0def5c 100644
--- a/src/application/Context/State/Code/Generation/Languages/BatchBuilder.ts
+++ b/src/application/Context/State/Code/Generation/Languages/BatchBuilder.ts
@@ -5,6 +5,12 @@ export class BatchBuilder extends CodeBuilder {
         return '::';
     }
     protected writeStandardOut(text: string): string {
-        return `echo ${text}`;
+        return `echo ${escapeForEcho(text)}`;
     }
 }
+
+function escapeForEcho(text: string) {
+    return text
+        .replace(/&/g, '^&')
+        .replace(/%/g, '%%');
+}
diff --git a/src/application/Context/State/Code/Generation/Languages/ShellBuilder.ts b/src/application/Context/State/Code/Generation/Languages/ShellBuilder.ts
index b8209569..5c45fac0 100644
--- a/src/application/Context/State/Code/Generation/Languages/ShellBuilder.ts
+++ b/src/application/Context/State/Code/Generation/Languages/ShellBuilder.ts
@@ -5,6 +5,11 @@ export class ShellBuilder extends CodeBuilder {
         return '#';
     }
     protected writeStandardOut(text: string): string {
-        return `echo '${text}'`;
+        return `echo '${escapeForEcho(text)}'`;
     }
 }
+
+function escapeForEcho(text: string) {
+    return text
+        .replace(/'/g, '\'\\\'\'');
+}
diff --git a/tests/unit/application/Context/State/Code/Generation/Languages/BatchBuilder.spec.ts b/tests/unit/application/Context/State/Code/Generation/Languages/BatchBuilder.spec.ts
index 394a42d7..8d7a5524 100644
--- a/tests/unit/application/Context/State/Code/Generation/Languages/BatchBuilder.spec.ts
+++ b/tests/unit/application/Context/State/Code/Generation/Languages/BatchBuilder.spec.ts
@@ -23,15 +23,37 @@ describe('BatchBuilder', () => {
         });
     });
     describe('writeStandardOut', () => {
-        it('prepends expected', () => {
-            // arrange
-            const text = 'test';
-            const expected = `echo ${text}`;
-            const sut = new BatchBuilderRevealer();
-            // act
-            const actual = sut.writeStandardOut(text);
-            // assert
-            expect(expected).to.equal(actual);
-        });
+        const testData = [
+            {
+                name: 'plain text',
+                text: 'test',
+                expected: 'echo test',
+            },
+            {
+                name: 'text with ampersand',
+                text: 'a & b',
+                expected: 'echo a ^& b',
+            },
+            {
+                name: 'text with percent sign',
+                text: '90%',
+                expected: 'echo 90%%',
+            },
+            {
+                name: 'text with multiple ampersands and percent signs',
+                text: 'Me&you in % ? You & me = 0%',
+                expected: 'echo Me^&you in %% ? You ^& me = 0%%',
+            },
+        ];
+        for (const test of testData) {
+            it(test.name, () => {
+                // arrange
+                const sut = new BatchBuilderRevealer();
+                // act
+                const actual = sut.writeStandardOut(test.text);
+                // assert
+                expect(test.expected).to.equal(actual);
+            });
+        }
     });
 });
diff --git a/tests/unit/application/Context/State/Code/Generation/Languages/ShellBuilder.spec.ts b/tests/unit/application/Context/State/Code/Generation/Languages/ShellBuilder.spec.ts
index 18929a9e..a21c6168 100644
--- a/tests/unit/application/Context/State/Code/Generation/Languages/ShellBuilder.spec.ts
+++ b/tests/unit/application/Context/State/Code/Generation/Languages/ShellBuilder.spec.ts
@@ -23,15 +23,32 @@ describe('ShellBuilder', () => {
         });
     });
     describe('writeStandardOut', () => {
-        it('prepends expected', () => {
-            // arrange
-            const text = 'test';
-            const expected = `echo '${text}'`;
-            const sut = new ShellBuilderRevealer();
-            // act
-            const actual = sut.writeStandardOut(text);
-            // assert
-            expect(expected).to.equal(actual);
-        });
+        const testData = [
+            {
+                name: 'plain text',
+                text: 'test',
+                expected: 'echo \'test\'',
+            },
+            {
+                name: 'text with single quote',
+                text: 'I\'m not who you think I am',
+                expected: 'echo \'I\'\\\'\'m not who you think I am\'',
+            },
+            {
+                name: 'text with multiple single quotes',
+                text: 'I\'m what you\'re',
+                expected: 'echo \'I\'\\\'\'m what you\'\\\'\'re\'',
+            },
+        ];
+        for (const test of testData) {
+            it(test.name, () => {
+                // arrange
+                const sut = new ShellBuilderRevealer();
+                // act
+                const actual = sut.writeStandardOut(test.text);
+                // assert
+                expect(test.expected).to.equal(actual);
+            });
+        }
     });
 });