diff --git a/lua/arti-pack/spec.lua b/lua/arti-pack/spec.lua index 69af109..ae6d5c3 100644 --- a/lua/arti-pack/spec.lua +++ b/lua/arti-pack/spec.lua @@ -9,6 +9,13 @@ local function trim(s) return (s:gsub('^%s+', ''):gsub('%s+$', '')) end +-- vim.system hands argv to the OS as C strings, so a NUL truncates the value +-- before curl sees it: the URL fetched would not be the one validated, locked +-- and shown. Lua's %s class does not cover it, so it needs its own check. +local function has_nul(s) + return s:find('\0', 1, true) ~= nil +end + local function bad_name(name) return name == '' or name == '.' or name == '..' or name:find('[/\\%s]') ~= nil end @@ -44,6 +51,9 @@ function M.parse(entry) return nil, string.format('%s is missing a tag or a branch', source) end ref = trim(ref) + if has_nul(ref) then + return nil, string.format('%s has a NUL byte in its %s %s', source, kind, vim.inspect(ref)) + end if bad_name(ref) then return nil, string.format('%s has an invalid %s %q', source, kind, ref) end @@ -54,6 +64,9 @@ function M.parse(entry) end local name = entry.as or repo + if type(name) == 'string' and has_nul(name) then + return nil, string.format('%s has a NUL byte in its directory name %s', source, vim.inspect(name)) + end if type(name) ~= 'string' or bad_name(name) then return nil, string.format('%s resolves to an unusable directory name %q', source, tostring(name)) end @@ -64,6 +77,9 @@ function M.parse(entry) return nil, string.format('%s has a non-string url', source) end url = trim(url) + if has_nul(url) then + return nil, string.format('%s has a NUL byte in its url %s', source, vim.inspect(url)) + end if url == '' or url:find('%s') then return nil, string.format('%s has an invalid url %q', source, entry.url) end @@ -96,6 +112,9 @@ function M.check_registry(registry) end -- Whitespace left inside the URL would be baked into every download. local base = trim(registry) + if has_nul(base) then + return nil, string.format('registry %s has a NUL byte in it; %s', vim.inspect(registry), SETUP_HINT) + end if not base:match('^https?://[^%s/]') or base:find('%s') then return nil, bad end diff --git a/tests/spec_spec.lua b/tests/spec_spec.lua index e3618e0..5f7554c 100644 --- a/tests/spec_spec.lua +++ b/tests/spec_spec.lua @@ -2,6 +2,11 @@ local t = require('tests.harness') local spec = require('arti-pack.spec') local REGISTRY = 'https://artifacts.example.com/artifactory/github-remote' +local NUL = string.char(0) + +-- A NUL truncates the value in argv, so anything that reaches curl must be +-- rejected before it can diverge from what was validated and locked. +local NULS = { 'mid' .. NUL .. 'dle', 'trailing' .. NUL, NUL } t.describe('spec.parse', function() t.it('parses a full table spec', function() @@ -131,6 +136,47 @@ t.describe('spec.parse', function() t.eq('https://example.test/a.zip', s.url) end) + t.it('rejects a tag carrying a NUL byte', function() + for _, bad in ipairs(NULS) do + local s, err = spec.parse({ 'owner/repo', tag = bad }) + t.falsy(s, 'rejects ' .. vim.inspect(bad)) + t.matches('NUL byte in its tag', err) + end + end) + + t.it('rejects a branch carrying a NUL byte', function() + for _, bad in ipairs(NULS) do + local s, err = spec.parse({ 'owner/repo', branch = bad }) + t.falsy(s, 'rejects ' .. vim.inspect(bad)) + t.matches('NUL byte in its branch', err) + end + end) + + t.it('rejects an "as" name carrying a NUL byte', function() + for _, bad in ipairs(NULS) do + local s, err = spec.parse({ 'owner/repo', tag = 'v1', as = bad }) + t.falsy(s, 'rejects ' .. vim.inspect(bad)) + t.matches('NUL byte in its directory name', err) + end + end) + + t.it('rejects a url carrying a NUL byte', function() + local urls = { 'https://example.test/a' .. NUL .. 'b.zip', 'https://example.test/a.zip' .. NUL, NUL } + for _, bad in ipairs(urls) do + local s, err = spec.parse({ 'owner/repo', tag = 'v1', url = bad }) + t.falsy(s, 'rejects ' .. vim.inspect(bad)) + t.matches('NUL byte in its url', err) + end + end) + + t.it('rejects a source carrying a NUL byte', function() + for _, bad in ipairs(NULS) do + local s, err = spec.parse({ 'owner/' .. bad, tag = 'v1' }) + t.falsy(s, 'rejects ' .. vim.inspect(bad)) + t.matches('expected "owner/repo"', err) + end + end) + t.it('rejects a url with whitespace inside it', function() for _, bad in ipairs({ 'https://example.test/a b.zip', 'https://example\n.test/a.zip', ' ' }) do local s, err = spec.parse({ 'owner/repo', tag = 'v1', url = bad }) @@ -242,6 +288,15 @@ t.describe('spec.check_registry', function() end end) + t.it('rejects a registry carrying a NUL byte', function() + local bads = { 'https://artifacts.example.com/g' .. NUL .. 'h', REGISTRY .. NUL, NUL } + for _, bad in ipairs(bads) do + local base, err = spec.check_registry(bad) + t.falsy(base, 'rejects ' .. vim.inspect(bad)) + t.matches('NUL byte', err) + end + end) + t.it('rejects a whitespace-only registry', function() for _, bad in ipairs({ ' ', ' ', '\n', ' \t\n ' }) do local base, err = spec.check_registry(bad)