Skip to content

Commit f13cd83

Browse files
authored
fix: check file existence after path resolution (#157)
fixes GHSA-587h-7wmj-h5pm
1 parent f6e6701 commit f13cd83

3 files changed

Lines changed: 55 additions & 19 deletions

File tree

‎packages/launch-editor/index.js‎

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ const childProcess = require('child_process')
1616
const guessEditor = require('./guess')
1717
const getArgumentsForFile = require('./get-args')
1818
const constructWindowsLaunchCommand = require('./construct-windows-launch-command')
19+
const parseFile = require('./parse-file')
1920

2021
function wrapErrorCallback(cb) {
2122
return (fileName, errorMessage) => {
@@ -42,24 +43,6 @@ function isTerminalEditor(editor) {
4243
return false
4344
}
4445

45-
const positionRE = /:(\d+)(:(\d+))?$/
46-
function parseFile(file) {
47-
// support `file://` protocol
48-
if (file.startsWith('file://')) {
49-
file = require('url').fileURLToPath(file)
50-
}
51-
52-
const fileName = file.replace(positionRE, '')
53-
const match = file.match(positionRE)
54-
const lineNumber = match && match[1]
55-
const columnNumber = match && match[3]
56-
return {
57-
fileName,
58-
lineNumber,
59-
columnNumber,
60-
}
61-
}
62-
6346
let currentChildProcess = null
6447

6548
function launchEditor(file, specifiedEditor, onErrorCallback) {
@@ -72,7 +55,7 @@ function launchEditor(file, specifiedEditor, onErrorCallback) {
7255

7356
onErrorCallback = wrapErrorCallback(onErrorCallback)
7457

75-
if (process.platform === 'win32' && path.resolve(fileName).startsWith('\\\\')) {
58+
if (process.platform === 'win32' && fileName.startsWith('\\\\')) {
7659
return onErrorCallback(
7760
fileName,
7861
'UNC paths are not supported on Windows to avoid security issues. ' +
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
const path = require('path')
2+
3+
const positionRE = /:(\d+)(:(\d+))?$/
4+
5+
module.exports = function parseFile(file) {
6+
// support `file://` protocol
7+
if (file.startsWith('file://')) {
8+
file = require('url').fileURLToPath(file)
9+
}
10+
11+
const unresolvedFileName = file.replace(positionRE, '')
12+
const fileName = unresolvedFileName ? path.resolve(unresolvedFileName) : ''
13+
14+
const match = file.match(positionRE)
15+
const lineNumber = match && match[1]
16+
const columnNumber = match && match[3]
17+
18+
return {
19+
fileName,
20+
lineNumber,
21+
columnNumber,
22+
}
23+
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
const assert = require('node:assert/strict')
2+
const { describe, test } = require('node:test')
3+
const path = require('node:path')
4+
5+
const parseFile = require('./parse-file.js')
6+
7+
describe('launchEditor path resolution', () => {
8+
test('does not resolve an empty file name to the working directory', () => {
9+
assert.equal(parseFile(':1:1').fileName, '')
10+
})
11+
12+
test('resolves relative file names', () => {
13+
assert.deepEqual(parseFile(`.${path.sep}file.js:1:1`), {
14+
fileName: path.resolve('file.js'),
15+
lineNumber: '1',
16+
columnNumber: '1',
17+
})
18+
})
19+
20+
test('resolves a parent-directory component created by a position suffix', () => {
21+
const project = path.resolve('project')
22+
const file = `${project}${path.sep}attacker-controlled${path.sep}..:1:1`
23+
24+
assert.deepEqual(parseFile(file), {
25+
fileName: project,
26+
lineNumber: '1',
27+
columnNumber: '1',
28+
})
29+
})
30+
})

0 commit comments

Comments
 (0)