skull-browser

@@ -308,17 +308,17 @@ return String(string).replace(/[&<>"'`=\/]/g, s => entityMap[s]);         }
 
         return `
-            <div class=bookmark data-id="${b.id}">
+            <div class=bookmark data-id="${escapeHTML(b.id)}">
                 <div class=title>
-                    <a href="${b.uri}">${escapeHTML(b.title || b.uri)}</a>
+                    <a href="${escapeHTML(b.uri)}">${escapeHTML(b.title || b.uri)}</a>
                 </div>
-                <div class=uri style="${b.title ? 'display: block;' : ''}">${b.uri}</div>
+                <div class=uri style="${b.title ? 'display: block;' : ''}">${escapeHTML(b.uri)}</div>
                 <div class=desc style="${b.markdown_desc ? 'display: block;' : ''}">
                     ${b.markdown_desc}
                 </div>
                 <div class=bottom>
-                    <span class=date>${b.date}</span>
-                    <span class=tags>${ tagArray.map(tag => '<a href=#>'+tag+'</a>').join('') }</span>
+                    <span class=date>${escapeHTML(b.date)}</span>
+                    <span class=tags>${ tagArray.map(tag => '<a href=#>'+escapeHTML(tag)+'</a>').join('') }</span>
                     <span class=controls>
                         <a href=# class=edit>edit</a>
                         <a href=# class=delete>delete</a>

@@ -133,15 +133,15 @@ return String(string).replace(/[&<>"'`=\/]/g, s => entityMap[s]);     }
 
     return `
-        <div class=download data-id="${d.id}" data-created="${d.created}">
-            <div class=date>${dateStr}</div>
+        <div class=download data-id="${escapeHTML(d.id)}" data-created="${escapeHTML(d.created)}">
+            <div class=date>${escapeHTML(dateStr)}</div>
             <div class=details>
                 <div class=title>
                     <a href="file://${escape(d.destination)}">${escapeHTML(href)}</a>
-                    <div class=status>${status_text}</div>
+                    <div class=status>${escapeHTML(status_text)}</div>
                 </div>
                 <div class=uri>
-                    <a href="${uri}">${uri}</a>
+                    <a href="${escapeHTML(uri)}">${escapeHTML(uri)}</a>
                 </div>
             </div>
             <div class=controls>

@@ -173,13 +173,13 @@ return String(string).replace(/[&<>"'`=\/]/g, s => entityMap[s]);         }
 
         return `
-            <div class=item data-id="${h.id}">
-                <span class=time>${h.time}</span>
+            <div class=item data-id="${escapeHTML(h.id)}">
+                <span class=time>${escapeHTML(h.time)}</span>
                 <span class=title>
-                    <a href="${h.uri}">${escapeHTML(h.title || h.uri)}</a>
+                    <a href="${escapeHTML(h.uri)}">${escapeHTML(h.title || h.uri)}</a>
                 </span>
                 <span class=domain>
-                    <a href=#>${domain}</a>
+                    <a href=#>${escapeHTML(domain)}</a>
                 </span>
             </div>
         `

@@ -121,13 +121,16 @@ </tr>     ]==]):gsub("\n +", ""):gsub("^ +", ""):gsub(" +$", "")
 local build_log_entry_html = function (entry)
     assert(entry)
+    local escape = lousy.util.escape
+    -- Log lines quote uris and server replies, so they are page controlled
+    -- even though the log itself is ours.
     return log_entry_fmt:gsub("{(%w+)}", {
         time = string.format("%012f", entry.time),
-        llevel = entry.level,
-        level = entry.level:gsub("^%l", string.upper),
-        group = entry.group,
-        groupkey = entry.group:gsub("/","-"),
-        msg = entry.msg,
+        llevel = escape(entry.level),
+        level = escape((entry.level:gsub("^%l", string.upper))),
+        group = escape(entry.group),
+        groupkey = escape((entry.group:gsub("/","-"))),
+        msg = escape(entry.msg),
     })
 end
 

@@ -410,6 +410,22 @@ function _M.sql_escape(s)     return "'" .. rstring.gsub(s or "", "'", "''") .. "'"
 end
 
+--- Escape a value for a shell command line.
+--
+-- Wraps the string in single quotes and rewrites any single quote it holds
+-- as <code>'\''</code>, which closes the quoted run, passes one literal
+-- quote, and opens the next run.
+--
+-- `luakit.spawn` splits its argument the way a shell would before handing
+-- the pieces to exec, so even a command that never reaches a shell needs
+-- its arguments quoted.
+--
+-- @tparam string s A string.
+-- @treturn string The quoted string, ready to be concatenated into a command.
+function _M.shell_escape(s)
+    return "'" .. rstring.gsub(tostring(s or ""), "'", "'\\''") .. "'"
+end
+
 --- Escape values for lua patterns.
 --
 -- Escapes the magic characters <code>^$()%.[]*+-?)</code> by prepending a

@@ -53,12 +53,12 @@ {             value = i.checked;
             let span = root.querySelector(".input > label > span");
             span.dataset.value = value;
-            span.innerHTML = value ? "Enabled" : "Disabled";
+            span.textContent = value ? "Enabled" : "Disabled";
         } else
             value = i.value;
         set_setting(key, value, type).then(function(error) {
             root.classList.toggle("has-error", error)
-            root.querySelector(".error-message").innerHTML = error;
+            root.querySelector(".error-message").textContent = error;
         });
     }
     function on_click (event) {

@@ -369,9 +369,24 @@ -- @tparam table guard a table that controls the watch process. Set `guard[1] -- = nil` to turn off the watch.
 -- @tparam string path the path of the watched style.
 _M.watch_styles = function (guard, path)
-    luakit.spawn(string.format("bash -c 'inotifywait -t 10 %q || sleep 1'", path), function ()
+    local function again ()
         _M.detect_files()
         if guard[1] then _M.watch_styles(guard, path) end
+    end
+
+    -- This used to run through `bash -c`, interpolating the path with Lua's
+    -- %q, which leaves the single quote alone. A stylesheet whose name held
+    -- one closed the quoting and the rest of the name ran as a command.
+    luakit.spawn("inotifywait -t 10 " .. lousy.util.shell_escape(path), function (_, status)
+        if status == 0 then return again() end
+        -- inotifywait timed out or is not installed: pause before looking
+        -- again, so a missing tool does not spin the loop.
+        local retry = timer{ interval = 1000 }
+        retry:add_signal("timeout", function ()
+            retry:stop()
+            again()
+        end)
+        retry:start()
     end)
 end
 

@@ -88,6 +88,30 @@ assert.equal(" %" .. ch .. " ", lousy.util.lua_escape(" " .. ch .. " "))     end
 end
 
+T.test_lousy_util_shell_escape = function ()
+    -- luakit.spawn and luakit.spawn_sync both run the command through
+    -- g_shell_parse_argv, so this checks the real property: whatever goes in
+    -- comes back out as one argument, unchanged and never interpreted.
+    local hostile = {
+        "plain.css",
+        "with space.css",
+        "it's.css",
+        "'; touch /tmp/skull-shell-escape-failed; echo '.css",
+        '"; rm -rf /; echo "',
+        "back\\slash.css",
+        "$HOME/`id`/${PATH}.css",
+        "semi;colon|pipe&amp.css",
+    }
+
+    for _, path in ipairs(hostile) do
+        local _, out = luakit.spawn_sync("printf %s " .. lousy.util.shell_escape(path))
+        assert.equal(path, out)
+    end
+
+    -- The payload above would have created this file had the quoting failed.
+    assert.is_nil(lfs.attributes("/tmp/skull-shell-escape-failed"))
+end
+
 return T
 
 -- vim: et:sw=4:ts=8:sts=4:tw=80