Skip to content

Commit

Permalink
fix(checkout): throw error on malicious filepaths (#1339)
Browse files Browse the repository at this point in the history
  • Loading branch information
billiegoose committed Apr 12, 2021
1 parent 89c0da7 commit 1316820
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 0 deletions.
14 changes: 14 additions & 0 deletions src/errors/UnsafeFilepathError.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { BaseError } from './BaseError.js'

export class UnsafeFilepathError extends BaseError {
/**
* @param {string} filepath
*/
constructor(filepath) {
super(`The filepath "${filepath}" contains unsafe character sequences`)
this.code = this.name = UnsafeFilepathError.code
this.data = { filepath }
}
}
/** @type {'UnsafeFilepathError'} */
UnsafeFilepathError.code = 'UnsafeFilepathError'
1 change: 1 addition & 0 deletions src/errors/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,6 @@ export * from './PushRejectedError.js'
export * from './RemoteCapabilityError.js'
export * from './SmartHttpError.js'
export * from './UnknownTransportError.js'
export * from './UnsafeFilepathError.js'
export * from './UrlParseError.js'
export * from './UserCanceledError.js'
7 changes: 7 additions & 0 deletions src/models/GitIndex.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { InternalError } from '../errors/InternalError.js'
import { UnsafeFilepathError } from '../errors/UnsafeFilepathError.js'
import { BufferCursor } from '../utils/BufferCursor.js'
import { comparePath } from '../utils/comparePath.js'
import { normalizeStats } from '../utils/normalizeStats.js'
Expand Down Expand Up @@ -92,6 +93,12 @@ export class GitIndex {
}
// TODO: handle pathnames larger than 12 bits
entry.path = reader.toString('utf8', pathlength)

// Prevent malicious paths like "..\foo"
if (entry.path.includes('..\\') || entry.path.includes('../')) {
throw new UnsafeFilepathError(entry.path)
}

// The next bit is awkward. We expect 1 to 8 null characters
// such that the total size of the entry is a multiple of 8 bits.
// (Hence subtract 12 bytes for the header.)
Expand Down
7 changes: 7 additions & 0 deletions src/models/GitTree.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { InternalError } from '../errors/InternalError.js'
import { UnsafeFilepathError } from '../errors/UnsafeFilepathError.js'
import { comparePath } from '../utils/comparePath.js'
import { compareTreeEntryPath } from '../utils/compareTreeEntryPath.js'

Expand Down Expand Up @@ -43,6 +44,12 @@ function parseBuffer(buffer) {
if (mode === '40000') mode = '040000' // makes it line up neater in printed output
const type = mode2type(mode)
const path = buffer.slice(space + 1, nullchar).toString('utf8')

// Prevent malicious git repos from writing to "..\foo" on clone etc
if (path.includes('\\') || path.includes('/')) {
throw new UnsafeFilepathError(path)
}

const oid = buffer.slice(nullchar + 1, nullchar + 21).toString('hex')
cursor = nullchar + 21
_entries.push({ mode, path, oid, type })
Expand Down

0 comments on commit 1316820

Please sign in to comment.