Skip to content

Commit

Permalink
fix: harden SRI parsing against ../ funny business
Browse files Browse the repository at this point in the history
The actual security fix this relates to is already fixed in cacache, but
defense in depth is a good and valuable thing.

BREAKING CHANGE: SRI values with `../` in the algorithm name now throw
as invalid (which they always probably should have!)
  • Loading branch information
isaacs committed Feb 18, 2020
1 parent a6811cb commit 4062735
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 2 deletions.
6 changes: 4 additions & 2 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@ const MiniPass = require('minipass')

const SPEC_ALGORITHMS = ['sha256', 'sha384', 'sha512']

// TODO: this should really be a hardcoded list of algorithms we support,
// rather than [a-z0-9].
const BASE64_REGEX = /^[a-z0-9+/]+(?:=?=?)$/i
const SRI_REGEX = /^([^-]+)-([^?]+)([?\S*]*)$/
const STRICT_SRI_REGEX = /^([^-]+)-([A-Za-z0-9+/=]{44,88})(\?[\x21-\x7E]*)*$/
const SRI_REGEX = /^([a-z0-9]+)-([^?]+)([?\S*]*)$/
const STRICT_SRI_REGEX = /^([a-z0-9]+)-([A-Za-z0-9+/=]{44,88})(\?[\x21-\x7E]*)*$/
const VCHAR_REGEX = /^[\x21-\x7E]+$/

const defaultOpts = {
Expand Down
10 changes: 10 additions & 0 deletions test/parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -226,3 +226,13 @@ test('supports strict spec parsing', t => {
}).toString(), valid, 'entries that fail strict check rejected')
t.done()
})

test('does not allow weird stuff in sri', t => {
const badInt = 'mdc2\u0000/../../../hello_what_am_I_doing_here-Juwtg9UFssfrRfwsXu+n/Q=='
const bad = ssri.parse(badInt)
const badStrict = ssri.parse(badInt, { strict: true })
const expect = ssri.parse('')
t.strictSame(bad, expect)
t.strictSame(badStrict, expect)
t.end()
})

0 comments on commit 4062735

Please sign in to comment.