Modifying the "dangerous download" algorithm.

chrome/browser/download/download_exe.cc

Index: chrome/browser/download/download_exe.cc
diff --git a/chrome/browser/download/download_exe.cc b/chrome/browser/download/download_exe.cc
index 8111f5d376eb997597ab07f48c0d4e61b10d1dd4..5e842ed38060056cf5c341d1dbd219a04c6d208d 100644
--- a/chrome/browser/download/download_exe.cc
+++ b/chrome/browser/download/download_exe.cc
@@ -56,137 +56,141 @@ namespace download_util {
  *
  * ***** END LICENSE BLOCK ***** */
 
-static const char* const g_executables[] = {
-  "class",
-  "htm",
-  "html",
-  "jar",
-  "pdf",
-  "pdfxml",
-  "mars",
-  "fdf",
-  "xfdf",
-  "xdp",
-  "xfd",
-  "pl",
-  "py",
-  "rb",
-  "shtm",
-  "shtml",
-  "svg",
-  "swf",
-  "xht",
-  "xhtm",
-  "xhtml",
-  "xml",
-  "xsl",
-  "xslt",
+static const struct Executables {

[bkr, 2010/11/17 02:30:04]

jnlp is pretty shady and should likely be included here.  It's included by
default with JRE 1.4 and >.  http://en.wikipedia.org/wiki/Java_Web_Start says it
all

+    const char* extension;
+    DownloadDangerLevel level;
+} g_executables[] = {
+  { "class", AllowOnUserGesture },
+  { "htm", AllowOnUserGesture },
+  { "html", AllowOnUserGesture },

[bkr, 2010/11/17 02:30:04]

Anything that is rendered as html (htm, html, shtm, etc.) is pretty dangerous. 
Running local attacker controlled html usually means arbitrary file/cookie theft
(I guess chrome has some protection against this).  Running attacker controlled
content also allows gives an attacker an opportunity to bypass any other file
extension restriction through the use of an EMBED or OBJECT tag.  Most plugins
will ignore extensions if explicitly called from an EMBED or OBJECT.

The security models of browser plugins is complicated, especially when being run
from a local context.  For example, if I run local html which loads a local SWF
I'll have local file access regardless of chrome local html restrictions.  Using
a local html file to load the local SWF means I can use an arbitrary extension
for the SWF file.  The same is true with JAR files (although the consequences
are greater with JAR)

[Peter Kasting, 2010/11/17 02:36:08]

It's at worst as dangerous as .exe and other "has to be manually run" types,
though, which means AllowOnUserGesture is sufficient.

The "Dangerous" value should be reserved for things that are dangerous just
sitting on the drive, like DLLs.

+  { "jar", AllowOnUserGesture },

[Chris Evans, 2010/11/19 02:55:54]

Mark jar (as well as jnlp) dangerous.

+  { "pdf", AllowOnUserGesture },
+  { "pdfxml", AllowOnUserGesture },
+  { "mars", AllowOnUserGesture },
+  { "fdf", AllowOnUserGesture },
+  { "xfdf", AllowOnUserGesture },
+  { "xdp", AllowOnUserGesture },
+  { "xfd", AllowOnUserGesture },
+  { "pl", AllowOnUserGesture },
+  { "py", AllowOnUserGesture },
+  { "rb", AllowOnUserGesture },

[Chris Evans, 2010/11/19 02:55:54]

.pl .py .rb and any other script extensions -> dangerous.

+  { "shtm", AllowOnUserGesture },
+  { "shtml", AllowOnUserGesture },
+  { "svg", AllowOnUserGesture },
+  { "swf", AllowOnUserGesture },
+  { "xht", AllowOnUserGesture },
+  { "xhtm", AllowOnUserGesture },
+  { "xhtml", AllowOnUserGesture },
+  { "xml", AllowOnUserGesture },
+  { "xsl", AllowOnUserGesture },
+  { "xslt", AllowOnUserGesture },
 #if defined(OS_WIN)

[bkr, 2010/11/17 02:30:04]

chm (compiled help files)
chi (collection/index of chms)
lnk <--like shortcuts... but better ;)
mmc (management console files)

[Peter Kasting, 2010/11/17 02:36:08]

That one is present already.

-  "ad",
-  "ade",
-  "adp",
-  "app",
-  "application",
-  "asp",
-  "asx",
-  "bas",
-  "bat",
-  "chm",
-  "cmd",
-  "com",
-  "cpl",
-  "crt",
-  "dll",
-  "exe",
-  "fxp",
-  "hlp",
-  "hta",
-  "htt",
-  "inf",
-  "ins",
-  "isp",
-  "js",
-  "jse",
-  "lnk",
-  "mad",
-  "maf",
-  "mag",
-  "mam",
-  "maq",
-  "mar",
-  "mas",
-  "mat",
-  "mau",
-  "mav",
-  "maw",
-  "mda",
-  "mdb",
-  "mde",
-  "mdt",
-  "mdw",
-  "mdz",
-  "mht",
-  "mhtml",
-  "msc",
-  "msh",
-  "mshxml",
-  "msi",
-  "msp",
-  "mst",
-  "ocx",
-  "ops",
-  "pcd",
-  "pif",
-  "plg",
-  "prf",
-  "prg",
-  "pst",
-  "reg",
-  "scf",
-  "scr",
-  "sct",
-  "shb",
-  "shs",
-  "url",
-  "vb",
-  "vbe",
-  "vbs",
-  "vsd",
-  "vsmacros",
-  "vss",
-  "vst",
-  "vsw",
-  "ws",
-  "wsc",
-  "wsf",
-  "wsh",
-  "xbap",
+  { "ad", AllowOnUserGesture },
+  { "ade", AllowOnUserGesture },
+  { "adp", AllowOnUserGesture },
+  { "app", AllowOnUserGesture },
+  { "application", AllowOnUserGesture },
+  { "asp", AllowOnUserGesture },
+  { "asx", AllowOnUserGesture },
+  { "bas", AllowOnUserGesture },
+  { "bat", AllowOnUserGesture },

[Chris Evans, 2010/11/19 02:55:54]

bat -> dangerous surely?

+  { "chm", AllowOnUserGesture },
+  { "cmd", AllowOnUserGesture },
+  { "com", AllowOnUserGesture },

[Chris Evans, 2010/11/19 02:55:54]

com == exe, no? -> dangerous!!

+  { "cpl", AllowOnUserGesture },
+  { "crt", AllowOnUserGesture },
+  { "dll", Dangerous },
+  { "exe", AllowOnUserGesture },

[Chris Evans, 2010/11/19 02:55:54]

Yowch. exe is terribly dangerous :)

+  { "fxp", AllowOnUserGesture },
+  { "hlp", AllowOnUserGesture },
+  { "hta", AllowOnUserGesture },
+  { "htt", AllowOnUserGesture },
+  { "inf", AllowOnUserGesture },
+  { "ins", AllowOnUserGesture },
+  { "isp", AllowOnUserGesture },
+  { "js", AllowOnUserGesture },
+  { "jse", AllowOnUserGesture },
+  { "lnk", AllowOnUserGesture },
+  { "mad", AllowOnUserGesture },
+  { "maf", AllowOnUserGesture },
+  { "mag", AllowOnUserGesture },
+  { "mam", AllowOnUserGesture },
+  { "maq", AllowOnUserGesture },
+  { "mar", AllowOnUserGesture },
+  { "mas", AllowOnUserGesture },
+  { "mat", AllowOnUserGesture },
+  { "mau", AllowOnUserGesture },
+  { "mav", AllowOnUserGesture },
+  { "maw", AllowOnUserGesture },
+  { "mda", AllowOnUserGesture },
+  { "mdb", AllowOnUserGesture },
+  { "mde", AllowOnUserGesture },
+  { "mdt", AllowOnUserGesture },
+  { "mdw", AllowOnUserGesture },
+  { "mdz", AllowOnUserGesture },
+  { "mht", AllowOnUserGesture },

[Chris Evans, 2010/11/19 02:55:54]

I don't know what half of these Windows things are. We need a Windows security
expert to cast an eye over the list before we land this.

+  { "mhtml", AllowOnUserGesture },
+  { "msc", AllowOnUserGesture },
+  { "msh", AllowOnUserGesture },
+  { "mshxml", AllowOnUserGesture },
+  { "msi", AllowOnUserGesture },
+  { "msp", AllowOnUserGesture },
+  { "mst", AllowOnUserGesture },
+  { "ocx", AllowOnUserGesture },
+  { "ops", AllowOnUserGesture },
+  { "pcd", AllowOnUserGesture },
+  { "pif", AllowOnUserGesture },
+  { "plg", AllowOnUserGesture },
+  { "prf", AllowOnUserGesture },
+  { "prg", AllowOnUserGesture },
+  { "pst", AllowOnUserGesture },
+  { "reg", AllowOnUserGesture },
+  { "scf", AllowOnUserGesture },
+  { "scr", AllowOnUserGesture },
+  { "sct", AllowOnUserGesture },
+  { "shb", AllowOnUserGesture },
+  { "shs", AllowOnUserGesture },
+  { "url", AllowOnUserGesture },
+  { "vb", AllowOnUserGesture },
+  { "vbe", AllowOnUserGesture },
+  { "vbs", AllowOnUserGesture },
+  { "vsd", AllowOnUserGesture },
+  { "vsmacros", AllowOnUserGesture },
+  { "vss", AllowOnUserGesture },
+  { "vst", AllowOnUserGesture },
+  { "vsw", AllowOnUserGesture },
+  { "ws", AllowOnUserGesture },
+  { "wsc", AllowOnUserGesture },
+  { "wsf", AllowOnUserGesture },
+  { "wsh", AllowOnUserGesture },
+  { "xbap", AllowOnUserGesture },

[Chris Evans, 2010/11/19 02:55:54]

xbap is a disaster! -> dangerous

 #elif defined(OS_MACOSX)
   // TODO(thakis): Figure out what makes sense here -- crbug.com/19096
-  "app",
-  "dmg",
+  { "app", AllowOnUserGesture },
+  { "dmg", AllowOnUserGesture },

[Chris Evans, 2010/11/19 02:55:54]

I don't know much about Mac. Does it adequately warn upon clicking on a
downloaded app / dmg in the download shelf?

 #elif defined(OS_POSIX)
   // TODO(estade): lengthen this list.
-  "bash",
-  "csh",
-  "deb",
-  "exe",
-  "ksh",
-  "rpm",
-  "sh",
-  "tcsh",
+  { "bash", AllowOnUserGesture },
+  { "csh", AllowOnUserGesture },
+  { "deb", AllowOnUserGesture },
+  { "exe", AllowOnUserGesture },

[Chris Evans, 2010/11/19 02:55:54]

exe seems to be repeated here?

+  { "ksh", AllowOnUserGesture },
+  { "rpm", AllowOnUserGesture },
+  { "sh", AllowOnUserGesture },

[Chris Evans, 2010/11/19 02:55:54]

.sh is certainly dangeous, as is .ksh, .tcsh, etc.
Plus, what about a Windows system with Cygwin installed? Does that add file
mappings for .sh etc. on the Windows system?

+  { "tcsh", AllowOnUserGesture },
 #endif
 };
 
-bool IsExecutableFile(const FilePath& path) {
-  return IsExecutableExtension(path.Extension());
+DownloadDangerLevel GetFileDangerLevel(const FilePath& path) {
+  return GetFileExtensionDangerLevel(path.Extension());
 }
 
-bool IsExecutableExtension(const FilePath::StringType& extension) {
+DownloadDangerLevel GetFileExtensionDangerLevel(
+    const FilePath::StringType& extension) {
   if (extension.empty())
-    return false;
+    return NotDangerous;
   if (!IsStringASCII(extension))
-    return false;
+    return NotDangerous;
 #if defined(OS_WIN)
   std::string ascii_extension = WideToASCII(extension);
 #elif defined(OS_POSIX)
@@ -198,10 +202,18 @@ bool IsExecutableExtension(const FilePath::StringType& extension) {
     ascii_extension.erase(0, 1);
 
   for (size_t i = 0; i < arraysize(g_executables); ++i) {
-    if (LowerCaseEqualsASCII(ascii_extension, g_executables[i]))
-      return true;
+    if (LowerCaseEqualsASCII(ascii_extension, g_executables[i].extension))
+      return g_executables[i].level;
   }
-  return false;
+  return NotDangerous;
+}
+
+bool IsFileExtensionSafe(const FilePath::StringType& extension) {
+  return GetFileExtensionDangerLevel(extension) == NotDangerous;
+}
+
+bool IsFileSafe(const FilePath& path) {
+  return GetFileDangerLevel(path) == NotDangerous;
 }
 
 static const char* kExecutableWhiteList[] = {