style: Prevent exponential blowup of custom properties.

Put a hard cap on the value length instead of counting substitutions, because it
works best, see the comment.

Differential Revision: https://phabricator.services.mozilla.com/D13352
This commit is contained in:
Emilio Cobos Álvarez 2018-11-29 10:16:38 +00:00
parent f6b6825c31
commit dca315e648

View file

@ -283,18 +283,32 @@ impl VariableValue {
} }
} }
fn push( fn push<'i>(
&mut self, &mut self,
input: &Parser<'i, '_>,
css: &str, css: &str,
css_first_token_type: TokenSerializationType, css_first_token_type: TokenSerializationType,
css_last_token_type: TokenSerializationType, css_last_token_type: TokenSerializationType,
) { ) -> Result<(), ParseError<'i>> {
/// Prevent values from getting terribly big since you can use custom
/// properties exponentially.
///
/// This number (1MB) is somewhat arbitrary, but silly enough that no
/// sane page would hit it. We could limit by number of total
/// substitutions, but that was very easy to work around in practice
/// (just choose a larger initial value and boom).
const MAX_VALUE_LENGTH_IN_BYTES: usize = 1024 * 1024;
if self.css.len() + css.len() > MAX_VALUE_LENGTH_IN_BYTES {
return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
}
// This happens e.g. between two subsequent var() functions: // This happens e.g. between two subsequent var() functions:
// `var(--a)var(--b)`. // `var(--a)var(--b)`.
// //
// In that case, css_*_token_type is nonsensical. // In that case, css_*_token_type is nonsensical.
if css.is_empty() { if css.is_empty() {
return; return Ok(());
} }
self.first_token_type.set_if_nothing(css_first_token_type); self.first_token_type.set_if_nothing(css_first_token_type);
@ -307,21 +321,32 @@ impl VariableValue {
self.css.push_str("/**/") self.css.push_str("/**/")
} }
self.css.push_str(css); self.css.push_str(css);
self.last_token_type = css_last_token_type self.last_token_type = css_last_token_type;
Ok(())
} }
fn push_from( fn push_from<'i>(
&mut self, &mut self,
input: &Parser<'i, '_>,
position: (SourcePosition, TokenSerializationType), position: (SourcePosition, TokenSerializationType),
input: &Parser,
last_token_type: TokenSerializationType, last_token_type: TokenSerializationType,
) { ) -> Result<(), ParseError<'i>> {
self.push(input.slice_from(position.0), position.1, last_token_type) self.push(
input,
input.slice_from(position.0),
position.1,
last_token_type,
)
} }
fn push_variable(&mut self, variable: &ComputedValue) { fn push_variable<'i>(
&mut self,
input: &Parser<'i, '_>,
variable: &ComputedValue,
) -> Result<(), ParseError<'i>> {
debug_assert!(variable.references.is_empty()); debug_assert!(variable.references.is_empty());
self.push( self.push(
input,
&variable.css, &variable.css,
variable.first_token_type, variable.first_token_type,
variable.last_token_type, variable.last_token_type,
@ -727,6 +752,7 @@ fn substitute_all(custom_properties_map: &mut CustomPropertiesMap, environment:
// title=Tarjan%27s_strongly_connected_components_algorithm&oldid=801728495 // title=Tarjan%27s_strongly_connected_components_algorithm&oldid=801728495
/// Struct recording necessary information for each variable. /// Struct recording necessary information for each variable.
#[derive(Debug)]
struct VarInfo { struct VarInfo {
/// The name of the variable. It will be taken to save addref /// The name of the variable. It will be taken to save addref
/// when the corresponding variable is popped from the stack. /// when the corresponding variable is popped from the stack.
@ -741,6 +767,7 @@ fn substitute_all(custom_properties_map: &mut CustomPropertiesMap, environment:
} }
/// Context struct for traversing the variable graph, so that we can /// Context struct for traversing the variable graph, so that we can
/// avoid referencing all the fields multiple times. /// avoid referencing all the fields multiple times.
#[derive(Debug)]
struct Context<'a> { struct Context<'a> {
/// Number of variables visited. This is used as the order index /// Number of variables visited. This is used as the order index
/// when we visit a new unresolved variable. /// when we visit a new unresolved variable.
@ -941,7 +968,7 @@ fn substitute_references_in_value<'i>(
environment, environment,
)?; )?;
computed_value.push_from(position, &input, last_token_type); computed_value.push_from(&input, position, last_token_type)?;
Ok(computed_value) Ok(computed_value)
} }
@ -955,8 +982,8 @@ fn substitute_references_in_value<'i>(
/// ///
/// Return `Err(())` if `input` is invalid at computed-value time. /// Return `Err(())` if `input` is invalid at computed-value time.
/// or `Ok(last_token_type that was pushed to partial_computed_value)` otherwise. /// or `Ok(last_token_type that was pushed to partial_computed_value)` otherwise.
fn substitute_block<'i, 't>( fn substitute_block<'i>(
input: &mut Parser<'i, 't>, input: &mut Parser<'i, '_>,
position: &mut (SourcePosition, TokenSerializationType), position: &mut (SourcePosition, TokenSerializationType),
partial_computed_value: &mut ComputedValue, partial_computed_value: &mut ComputedValue,
custom_properties: &CustomPropertiesMap, custom_properties: &CustomPropertiesMap,
@ -991,10 +1018,11 @@ fn substitute_block<'i, 't>(
let is_env = name.eq_ignore_ascii_case("env"); let is_env = name.eq_ignore_ascii_case("env");
partial_computed_value.push( partial_computed_value.push(
input,
input.slice(position.0..before_this_token), input.slice(position.0..before_this_token),
position.1, position.1,
last_token_type, last_token_type,
); )?;
input.parse_nested_block(|input| { input.parse_nested_block(|input| {
// parse_var_function() / parse_env_function() ensure neither .unwrap() will fail. // parse_var_function() / parse_env_function() ensure neither .unwrap() will fail.
let name = { let name = {
@ -1014,7 +1042,7 @@ fn substitute_block<'i, 't>(
if let Some(v) = value { if let Some(v) = value {
last_token_type = v.last_token_type; last_token_type = v.last_token_type;
partial_computed_value.push_variable(v); partial_computed_value.push_variable(input, v)?;
// Skip over the fallback, as `parse_nested_block` would return `Err` // Skip over the fallback, as `parse_nested_block` would return `Err`
// if we don't consume all of `input`. // if we don't consume all of `input`.
// FIXME: Add a specialized method to cssparser to do this with less work. // FIXME: Add a specialized method to cssparser to do this with less work.
@ -1036,7 +1064,7 @@ fn substitute_block<'i, 't>(
custom_properties, custom_properties,
env, env,
)?; )?;
partial_computed_value.push_from(position, input, last_token_type); partial_computed_value.push_from(input, position, last_token_type)?;
} }
Ok(()) Ok(())
})?; })?;
@ -1096,6 +1124,6 @@ pub fn substitute<'i>(
&custom_properties, &custom_properties,
env, env,
)?; )?;
substituted.push_from(position, &input, last_token_type); substituted.push_from(&input, position, last_token_type)?;
Ok(substituted.css) Ok(substituted.css)
} }