Merge pull request from GHSA-7r3h-m5j6-3q42

* use uuid as our multiline env delimiter * remove extra fn * Fix version * also throw error if delimiter is found in name or value * move delimiter and uuid to global var in test * upgrade uuid to newest version * remove spy variable * Update packages/core/src/core.ts Co-authored-by: Thomas Boop <[email protected]> * Update packages/core/src/core.ts Co-authored-by: Thomas Boop <[email protected]>
RobertDeRose · Aug 8, 2022 · 4beda9c · 4beda9c
1 parent 90be12a
commit 4beda9c
Show file tree

Hide file tree

Showing 5 changed files with 92 additions and 11 deletions.
diff --git a/packages/core/RELEASES.md b/packages/core/RELEASES.md
@@ -1,5 +1,8 @@
 # @actions/core Releases
 
+### 1.9.1
+- Randomize delimiter when calling `core.exportVariable`
+
 ### 1.9.0
 - Added `toPosixPath`, `toWin32Path` and `toPlatformPath` utilities [#1102](https://github.com/actions/toolkit/pull/1102)
 

diff --git a/packages/core/__tests__/core.test.ts b/packages/core/__tests__/core.test.ts
@@ -4,6 +4,9 @@ import * as path from 'path'
 import * as core from '../src/core'
 import {HttpClient} from '@actions/http-client'
 import {toCommandProperties} from '../src/utils'
+import * as uuid from 'uuid'
+
+jest.mock('uuid')
 
 /* eslint-disable @typescript-eslint/unbound-method */
 
@@ -41,6 +44,9 @@ const testEnvVars = {
   GITHUB_ENV: ''
 }
 
+const UUID = '9b1deb4d-3b7d-4bad-9bdd-2b0d7b3dcb6d'
+const DELIMITER = `ghadelimiter_${UUID}`
+
 describe('@actions/core', () => {
   beforeAll(() => {
     const filePath = path.join(__dirname, `test`)
@@ -54,6 +60,14 @@ describe('@actions/core', () => {
       process.env[key] = testEnvVars[key as keyof typeof testEnvVars]
     }
     process.stdout.write = jest.fn()
+
+    jest.spyOn(uuid, 'v4').mockImplementation(() => {
+      return UUID
+    })
+  })
+
+  afterEach(() => {
+    jest.restoreAllMocks()
   })
 
   it('legacy exportVariable produces the correct command and sets the env', () => {
@@ -91,7 +105,7 @@ describe('@actions/core', () => {
     core.exportVariable('my var', 'var val')
     verifyFileCommand(
       command,
-      `my var<<_GitHubActionsFileCommandDelimeter_${os.EOL}var val${os.EOL}_GitHubActionsFileCommandDelimeter_${os.EOL}`
+      `my var<<${DELIMITER}${os.EOL}var val${os.EOL}${DELIMITER}${os.EOL}`
     )
   })
 
@@ -101,7 +115,7 @@ describe('@actions/core', () => {
     core.exportVariable('my var', true)
     verifyFileCommand(
       command,
-      `my var<<_GitHubActionsFileCommandDelimeter_${os.EOL}true${os.EOL}_GitHubActionsFileCommandDelimeter_${os.EOL}`
+      `my var<<${DELIMITER}${os.EOL}true${os.EOL}${DELIMITER}${os.EOL}`
     )
   })
 
@@ -111,10 +125,34 @@ describe('@actions/core', () => {
     core.exportVariable('my var', 5)
     verifyFileCommand(
       command,
-      `my var<<_GitHubActionsFileCommandDelimeter_${os.EOL}5${os.EOL}_GitHubActionsFileCommandDelimeter_${os.EOL}`
+      `my var<<${DELIMITER}${os.EOL}5${os.EOL}${DELIMITER}${os.EOL}`
     )
   })
 
+  it('exportVariable does not allow delimiter as value', () => {
+    const command = 'ENV'
+    createFileCommandFile(command)
+
+    expect(() => {
+      core.exportVariable('my var', `good stuff ${DELIMITER} bad stuff`)
+    }).toThrow(`Unexpected input: value should not contain the delimiter "${DELIMITER}"`)
+
+    const filePath = path.join(__dirname, `test/${command}`)
+    fs.unlinkSync(filePath)
+  })
+
+  it('exportVariable does not allow delimiter as name', () => {
+    const command = 'ENV'
+    createFileCommandFile(command)
+
+    expect(() => {
+      core.exportVariable(`good stuff ${DELIMITER} bad stuff`, 'test')
+    }).toThrow(`Unexpected input: name should not contain the delimiter "${DELIMITER}"`)
+
+    const filePath = path.join(__dirname, `test/${command}`)
+    fs.unlinkSync(filePath)
+  })
+
   it('setSecret produces the correct command', () => {
     core.setSecret('secret val')
     assertWriteCalls([`::add-mask::secret val${os.EOL}`])

diff --git a/packages/core/package-lock.json b/packages/core/package-lock.json
diff --git a/packages/core/package.json b/packages/core/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@actions/core",
-  "version": "1.9.0",
+  "version": "1.9.1",
   "description": "Actions core lib",
   "keywords": [
     "github",
@@ -36,9 +36,11 @@
     "url": "https://github.com/actions/toolkit/issues"
   },
   "dependencies": {
-    "@actions/http-client": "^2.0.1"
+    "@actions/http-client": "^2.0.1",
+    "uuid": "^8.3.2"
   },
   "devDependencies": {
-    "@types/node": "^12.0.2"
+    "@types/node": "^12.0.2",
+    "@types/uuid": "^8.3.4"
   }
 }
diff --git a/packages/core/src/core.ts b/packages/core/src/core.ts
@@ -4,6 +4,7 @@ import {toCommandProperties, toCommandValue} from './utils'
 
 import * as os from 'os'
 import * as path from 'path'
+import { v4 as uuidv4 } from 'uuid'
 
 import {OidcClient} from './oidc-utils'
 
@@ -86,7 +87,17 @@ export function exportVariable(name: string, val: any): void {
 
   const filePath = process.env['GITHUB_ENV'] || ''
   if (filePath) {
-    const delimiter = '_GitHubActionsFileCommandDelimeter_'
+    const delimiter = `ghadelimiter_${uuidv4()}`
+
+    // These should realistically never happen, but just in case someone finds a way to exploit uuid generation let's not allow keys or values that contain the delimiter.  
+    if (name.includes(delimiter)) {
+      throw new Error(`Unexpected input: name should not contain the delimiter "${delimiter}"`)
+    }
+
+    if (convertedVal.includes(delimiter)) {
+      throw new Error(`Unexpected input: value should not contain the delimiter "${delimiter}"`)
+    }
+
     const commandValue = `${name}<<${delimiter}${os.EOL}${convertedVal}${os.EOL}${delimiter}`
     issueFileCommand('ENV', commandValue)
   } else {