From 4f268b8ef17fda03fadf49c3d61e6445f84ce035 Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Wed, 24 Nov 2021 20:37:03 +0100
Subject: [PATCH] scaffolder-backend: use a const reference to helpers in the
 SecureTemplater

Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
---
 .../lib/templating/SecureTemplater.test.ts    | 29 +++++++++++++++++++
 .../src/lib/templating/SecureTemplater.ts     |  9 ++++--
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.test.ts b/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.test.ts
index 899f57e295..b83c9eea4f 100644
--- a/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.test.ts
+++ b/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.test.ts
@@ -113,4 +113,33 @@ describe('SecureTemplater', () => {
       ['https://my-host.com/my-owner/my-repo'],
     ]);
   });
+
+  it('should not allow helpers to be rewritten', async () => {
+    const templater = new SecureTemplater({
+      parseRepoUrl: () => ({
+        repo: 'my-repo',
+        owner: 'my-owner',
+        host: 'my-host.com',
+      }),
+    });
+    await templater.initializeIfNeeded();
+
+    const ctx = {
+      repoUrl: 'https://my-host.com/my-owner/my-repo',
+    };
+    expect(
+      templater.render(
+        '${{ ({}).constructor.constructor("parseRepoUrl = () => JSON.stringify(`inject`)")() }}',
+        ctx,
+      ),
+    ).toBe('');
+
+    expect(templater.render('${{ repoUrl | parseRepoUrl | dump }}', ctx)).toBe(
+      JSON.stringify({
+        repo: 'my-repo',
+        owner: 'my-owner',
+        host: 'my-host.com',
+      }),
+    );
+  });
 });
diff --git a/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts b/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts
index c4f14cfe3f..eba1c3838d 100644
--- a/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts
+++ b/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts
@@ -45,11 +45,13 @@ const { render, renderCompat } = (() => {
   compatEnv.addFilter('jsonify', compatEnv.getFilter('dump'));
 
   if (typeof parseRepoUrl !== 'undefined') {
+    const safeHelperRef = parseRepoUrl;
+
     env.addFilter('parseRepoUrl', repoUrl => {
-      return JSON.parse(parseRepoUrl(repoUrl))
+      return JSON.parse(safeHelperRef(repoUrl))
     });
     env.addFilter('projectSlug', repoUrl => {
-      const { owner, repo } = JSON.parse(parseRepoUrl(repoUrl));
+      const { owner, repo } = JSON.parse(safeHelperRef(repoUrl));
       return owner + '/' + repo;
     });
   }
@@ -132,6 +134,9 @@ export class SecureTemplater {
         };
       }
 
+      // Note that helpers in the sandbox can be rewritten with a script injection.
+      // In order to mitigate that each helper must be assigned to a constant variable
+      // inside the closure that houses the nunjucks implementation.
       this.#vm = new VM({ timeout: 1000, sandbox });
 
       const nunjucksSource = await fs.readFile(