Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(39)

Unified Diff: extensions/common/csp_validator.cc

Issue 2563843002: Restrict app sandbox's CSP to disallow loading web content in them. (Closed)
Patch Set: Created 4 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: extensions/common/csp_validator.cc
diff --git a/extensions/common/csp_validator.cc b/extensions/common/csp_validator.cc
index e6c789f6f8e59fce0d2afdc732efac15d82a37f6..409e6404114bd0c79df8c69b495c649264ea4299 100644
--- a/extensions/common/csp_validator.cc
+++ b/extensions/common/csp_validator.cc
@@ -28,12 +28,20 @@ namespace {
const char kDefaultSrc[] = "default-src";
const char kScriptSrc[] = "script-src";
const char kObjectSrc[] = "object-src";
+const char kFrameSrc[] = "frame-src";
+const char kChildSrc[] = "child-src";
+
+const char kDirectiveSeparator[] = ";";
+
const char kPluginTypes[] = "plugin-types";
const char kObjectSrcDefaultDirective[] = "object-src 'self';";
const char kScriptSrcDefaultDirective[] =
"script-src 'self' chrome-extension-resource:;";
+const char kAppSandboxSubframeSrcDefaultDirective[] = "child-src 'self';";
+const char kAppSandboxScriptSrcDefaultDirective[] = "script-src 'self';";
+
const char kSandboxDirectiveName[] = "sandbox";
const char kAllowSameOriginToken[] = "allow-same-origin";
const char kAllowTopNavigation[] = "allow-top-navigation";
@@ -276,7 +284,7 @@ std::string SanitizeContentSecurityPolicy(
std::vector<InstallWarning>* warnings) {
// See http://www.w3.org/TR/CSP/#parse-a-csp-policy for parsing algorithm.
std::vector<std::string> directives = base::SplitString(
- policy, ";", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
+ policy, kDirectiveSeparator, base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
DirectiveStatus default_src_status(kDefaultSrc);
DirectiveStatus script_src_status(kScriptSrc);
@@ -306,7 +314,7 @@ std::string SanitizeContentSecurityPolicy(
continue;
// Pass the other CSP directives as-is without further validation.
- sane_csp_parts.push_back(input + ";");
+ sane_csp_parts.push_back(input + kDirectiveSeparator);
}
if (default_src_status.seen_in_policy) {
@@ -337,6 +345,63 @@ std::string SanitizeContentSecurityPolicy(
return base::JoinString(sane_csp_parts, " ");
}
+std::string GetEffectiveSandoxedPageCSP(const std::string& policy) {
Devlin 2016/12/09 15:46:01 I kind of wonder if it makes sense to have a more
Charlie Reis 2016/12/09 19:55:07 I haven't seen one, but I agree that avoiding a bu
alexmos 2016/12/12 19:04:11 I haven't seen one either, and I agree it's a good
lazyboy 2016/12/14 00:49:05 I've introduced a CSPEnforcer class that can sanit
+ // See http://www.w3.org/TR/CSP/#parse-a-csp-policy for parsing algorithm.
+ std::vector<std::string> effective_csp_parts;
+ bool seen_default_source = false;
+ bool seen_subframe_source = false;
+ bool seen_script_source = false;
+ for (const std::string& directive : base::SplitString(
+ policy, ";", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL)) {
+ base::StringTokenizer tokenizer(directive, " \t\r\n");
+ if (!tokenizer.GetNext())
+ continue;
+
+ std::string directive_name = base::ToLowerASCII(tokenizer.token_piece());
+ if (directive_name == kDefaultSrc) {
+ seen_default_source = true;
+ } else if (directive_name == kChildSrc || directive_name == kFrameSrc) {
+ seen_subframe_source = true;
+ } else if (directive_name == kScriptSrc) {
+ seen_script_source = true;
+ } else {
+ // Keep this directive as is.
+ effective_csp_parts.push_back(directive + kDirectiveSeparator);
+ continue;
+ }
+
+ effective_csp_parts.push_back(directive_name);
+ bool seen_self_or_none = false;
+ while (tokenizer.GetNext()) {
+ std::string source_lower = base::ToLowerASCII(tokenizer.token());
+ seen_self_or_none |= source_lower == "'none'" || source_lower == "'self'";
+
+ // Keyword directive sources are surrounded with quotes, e.g. 'self',
+ // 'sha256-...', 'unsafe-eval', 'nonce-...'. These do not specify a remote
+ // host or '*', so keep them and restrict the rest.
+ if (source_lower.size() > 1u && source_lower[0] == '\'' &&
+ source_lower[source_lower.size() - 1] == '\'') {
Devlin 2016/12/09 15:46:01 When we see an insecure pattern, we could add an i
lazyboy 2016/12/14 00:49:05 Done.
+ effective_csp_parts.push_back(source_lower);
+ }
+ }
+
+ if (!seen_self_or_none)
+ effective_csp_parts.push_back("'self'");
+
+ // Add ";" at the end of the directive.
+ effective_csp_parts.back().append(kDirectiveSeparator);
+ }
+
+ // If subframes, scripts were not restricted in |effective_csp_parts|, add the
+ // restriction.
+ if (!seen_subframe_source && !seen_default_source)
+ effective_csp_parts.push_back(kAppSandboxSubframeSrcDefaultDirective);
+ if (!seen_script_source && !seen_default_source)
+ effective_csp_parts.push_back(kAppSandboxScriptSrcDefaultDirective);
+
+ return base::JoinString(effective_csp_parts, " ");
+}
+
bool ContentSecurityPolicyIsSandboxed(
const std::string& policy, Manifest::Type type) {
// See http://www.w3.org/TR/CSP/#parse-a-csp-policy for parsing algorithm.

Powered by Google App Engine
This is Rietveld 408576698