From: Jo-Philipp Wich <jo@mein.io>
Date: Sat, 7 Apr 2018 09:43:44 +0000 (+0200)
Subject: luci-base: escape path strings and field parameter
X-Git-Url: http://git.archive.openwrt.org/?p=project%2Fluci.git;a=commitdiff_plain;h=5c31937a0f0bf8fa2f0161cadae9688fff9c227e

luci-base: escape path strings and field parameter

Prevent various XSS vectors by not interpolating field and path values
verbatim into script and html contexts.

Signed-off-by: Jo-Philipp Wich <jo@mein.io>
---

diff --git a/modules/luci-base/luasrc/view/cbi/filebrowser.htm b/modules/luci-base/luasrc/view/cbi/filebrowser.htm
index a79beebba..806b1b5f4 100644
--- a/modules/luci-base/luasrc/view/cbi/filebrowser.htm
+++ b/modules/luci-base/luasrc/view/cbi/filebrowser.htm
@@ -22,9 +22,9 @@
 	<script type="text/javascript">
 		function callback(path) {
 			if( window.opener ) {
-				var input = window.opener.document.getElementById('<%=luci.http.formvalue('field')%>');
+				var input = window.opener.document.getElementById(decodeURIComponent('<%=luci.http.urlencode(luci.http.formvalue('field'))%>'));
 				if( input ) {
-					input.value = path;
+					input.value = decodeURIComponent(path);
 					window.close();
 				}
 			}
@@ -48,33 +48,44 @@
 			end
 		end
 
-		local filepath = table.concat( path, '/' )
-		local filestat = nixio.fs.stat( filepath )
-		local baseurl  = luci.dispatcher.build_url('admin', 'filebrowser')
+		local filestat = nixio.fs.stat(table.concat(path, '/'))
+		local baseurl  = { 'admin', 'filebrowser' }
 
 		if filestat and filestat.type == "reg" then
-			table.remove( path, #path )
-			filepath = table.concat( path, '/' ) .. '/'
-		elseif not ( filestat and filestat.type == "dir" ) then
-			path     = { '' }
-			filepath = '/'
+			path[#path] = ''
+		elseif not (filestat and filestat.type == "dir") then
+			path = { '', '' }
 		else
-			filepath = filepath .. '/'
+			path[#path+1] = ''
 		end
 
-		local entries = nixio.util.consume((nixio.fs.dir(filepath)))
+		filepath = table.concat(path, '/')
+
+		local entries = {}
+		local _, e
+		for _, e in luci.util.vspairs(nixio.util.consume((nixio.fs.dir(filepath)))) do
+			local p = filepath .. e
+			local s = nixio.fs.stat(p)
+			if s then
+				entries[#entries+1] = {
+					name = e,
+					path = p,
+					type = s.type
+				}
+			end
+		end
 	-%>
-    <div id="path">
+	<div id="path">
 		Location:
 		<% for i, dir in ipairs(path) do %>
 			<% if i == 1 then %>
-				<a href="<%=baseurl%>?field=<%=field%>">(root)</a>
+				<a href="<%=url(unpack(baseurl))%>?field=<%=luci.http.urlencode(field)%>">(root)</a>
 			<% elseif next(path, i) then %>
-				<% baseurl = baseurl .. '/' .. dir %>
-				/ <a href="<%=baseurl%>?field=<%=field%>"><%=dir%></a>
+				<% baseurl[#baseurl+1] = luci.http.urlencode(dir) %>
+				/ <a href="<%=url(unpack(baseurl))%>?field=<%=luci.http.urlencode(field)%>"><%=pcdata(dir)%></a>
 			<% else %>
-				<% baseurl = baseurl .. '/' .. dir %>
-				/ <%=dir%>
+				<% baseurl[#baseurl+1] = luci.http.urlencode(dir) %>
+				/ <%=pcdata(dir)%>
 			<% end %>
 		<% end %>
 	</div>
@@ -83,23 +94,17 @@
 
 	<div id="listing">
 		<ul>
-			<% for _, e in luci.util.vspairs(entries) do
-			    local stat = nixio.fs.stat(filepath..e)
-				if stat and stat.type == 'dir' then
-			-%>
+			<% for _, e in ipairs(entries) do if e.type == 'dir' then -%>
 				<li class="dir">
 					<img src="<%=resource%>/cbi/folder.gif" alt="<%:Directory%>" />
-					<a href="<%=baseurl%>/<%=e%>?field=<%=field%>"><%=e%>/</a>
+					<a href="<%=url(unpack(baseurl))%>/<%=luci.http.urlencode(e.name)%>?field=<%=luci.http.urlencode(field)%>"><%=pcdata(e.name)%>/</a>
 				</li>
 			<% end end -%>
 
-			<% for _, e in luci.util.vspairs(entries) do
-			    local stat = nixio.fs.stat(filepath..e)
-				if stat and stat.type ~= 'dir' then
-			-%>
+			<% for _, e in ipairs(entries) do if e.type ~= 'dir' then -%>
 				<li class="file">
 					<img src="<%=resource%>/cbi/file.gif" alt="<%:File%>" />
-					<a href="#" onclick="callback('<%=filepath..e%>')"><%=e%></a>
+					<a href="#" onclick="callback('<%=luci.http.urlencode(e.path)%>')"><%=pcdata(e.name)%></a>
 				</li>
 			<% end end -%>
 		</ul>