From e630f647d1fe53e75996af9089a31bec1853b7d8 Mon Sep 17 00:00:00 2001
From: Lorenzo Manacorda <asymmetric@users.noreply.github.com>
Date: Mon, 19 Feb 2018 10:36:34 +0100
Subject: [PATCH] spec: Validate required divisor fields are not 0 (#7933)

* Add validate_non_zero function

It's used to validate that a Spec's uint field used as a divisor is not zero.

* Add deserialize_with to gas_limit_bound_divisor

Prevents panics due to divide-by-zero on the gas_limit_bound_divisor
field.

* Add deserialize_with to difficulty_bound_divisor

Prevents panics due to divide-by-zero on the difficulty_bound_divisor
field.

* Add validate_optional_non_zero function

Used to validate Option<Uint> divisor fields.

* Use deserialize_with on optional divisor fields.

* Add #[serde(default)] attribute to divisor fields

When using `#[serde(deserialize_with)]`, `#[serde(default)]` must be specified so that missing
fields can be deserialized with the deserializer for `None`.
---
 json/src/spec/ethash.rs | 19 ++++++++++++++++++-
 json/src/spec/params.rs | 20 +++++++++++++++++++-
 json/src/uint.rs        | 24 +++++++++++++++++++++++-
 3 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/json/src/spec/ethash.rs b/json/src/spec/ethash.rs
index 03f6f6175..afa8b5968 100644
--- a/json/src/spec/ethash.rs
+++ b/json/src/spec/ethash.rs
@@ -16,7 +16,7 @@
 
 //! Ethash params deserialization.
 
-use uint::Uint;
+use uint::{self, Uint};
 use hash::Address;
 
 /// Deserializable doppelganger of EthashParams.
@@ -27,12 +27,15 @@ pub struct EthashParams {
 	pub minimum_difficulty: Uint,
 	/// See main EthashParams docs.
 	#[serde(rename="difficultyBoundDivisor")]
+	#[serde(deserialize_with="uint::validate_non_zero")]
 	pub difficulty_bound_divisor: Uint,
 	/// See main EthashParams docs.
 	#[serde(rename="difficultyIncrementDivisor")]
+	#[serde(default, deserialize_with="uint::validate_optional_non_zero")]
 	pub difficulty_increment_divisor: Option<Uint>,
 	/// See main EthashParams docs.
 	#[serde(rename="metropolisDifficultyIncrementDivisor")]
+	#[serde(default, deserialize_with="uint::validate_optional_non_zero")]
 	pub metropolis_difficulty_increment_divisor: Option<Uint>,
 	/// See main EthashParams docs.
 	#[serde(rename="durationLimit")]
@@ -60,6 +63,7 @@ pub struct EthashParams {
 	pub difficulty_hardfork_transition: Option<Uint>,
 	/// See main EthashParams docs.
 	#[serde(rename="difficultyHardforkBoundDivisor")]
+	#[serde(default, deserialize_with="uint::validate_optional_non_zero")]
 	pub difficulty_hardfork_bound_divisor: Option<Uint>,
 	/// See main EthashParams docs.
 	#[serde(rename="bombDefuseTransition")]
@@ -302,4 +306,17 @@ mod tests {
 			}
 		});
 	}
+
+	#[test]
+	#[should_panic(expected = "a non-zero value")]
+	fn test_zero_value_divisor() {
+		let s = r#"{
+			"params": {
+				"difficultyBoundDivisor": "0x0",
+				"minimumDifficulty": "0x020000"
+			}
+		}"#;
+
+		let _deserialized: Ethash = serde_json::from_str(s).unwrap();
+	}
 }
diff --git a/json/src/spec/params.rs b/json/src/spec/params.rs
index 3768d59c9..8e5fd9f00 100644
--- a/json/src/spec/params.rs
+++ b/json/src/spec/params.rs
@@ -16,7 +16,7 @@
 
 //! Spec params deserialization.
 
-use uint::Uint;
+use uint::{self, Uint};
 use hash::{H256, Address};
 use bytes::Bytes;
 
@@ -102,6 +102,7 @@ pub struct Params {
 	pub wasm: Option<bool>,
 	/// See `CommonParams` docs.
 	#[serde(rename="gasLimitBoundDivisor")]
+	#[serde(deserialize_with="uint::validate_non_zero")]
 	pub gas_limit_bound_divisor: Uint,
 	/// See `CommonParams` docs.
 	pub registrar: Option<Address>,
@@ -149,4 +150,21 @@ mod tests {
 		assert_eq!(deserialized.gas_limit_bound_divisor, Uint(U256::from(0x20)));
 		assert_eq!(deserialized.max_code_size, Some(Uint(U256::from(0x1000))));
 	}
+
+	#[test]
+	#[should_panic(expected = "a non-zero value")]
+	fn test_zero_value_divisor() {
+		let s = r#"{
+			"maximumExtraDataSize": "0x20",
+			"networkID" : "0x1",
+			"chainID" : "0x15",
+			"subprotocolName" : "exp",
+			"minGasLimit": "0x1388",
+			"accountStartNonce": "0x01",
+			"gasLimitBoundDivisor": "0x0",
+			"maxCodeSize": "0x1000"
+		}"#;
+
+		let _deserialized: Params = serde_json::from_str(s).unwrap();
+	}
 }
diff --git a/json/src/uint.rs b/json/src/uint.rs
index 81fa36a63..e919b6987 100644
--- a/json/src/uint.rs
+++ b/json/src/uint.rs
@@ -19,7 +19,7 @@
 use std::fmt;
 use std::str::FromStr;
 use serde::{Deserialize, Deserializer};
-use serde::de::{Error, Visitor};
+use serde::de::{Error, Visitor, Unexpected};
 use ethereum_types::U256;
 
 /// Lenient uint json deserialization for test json files.
@@ -90,6 +90,28 @@ impl<'a> Visitor<'a> for UintVisitor {
 	}
 }
 
+pub fn validate_non_zero<'de, D>(d: D) -> Result<Uint, D::Error> where D: Deserializer<'de> {
+	let value = Uint::deserialize(d)?;
+
+	if value == Uint(U256::from(0)) {
+		return Err(Error::invalid_value(Unexpected::Unsigned(value.into()), &"a non-zero value"))
+	}
+
+	Ok(value)
+}
+
+pub fn validate_optional_non_zero<'de, D>(d: D) -> Result<Option<Uint>, D::Error> where D: Deserializer<'de> {
+	let value: Option<Uint> = Option::deserialize(d)?;
+
+	if let Some(value) = value {
+		if value == Uint(U256::from(0)) {
+			return Err(Error::invalid_value(Unexpected::Unsigned(value.into()), &"a non-zero value"))
+		}
+	}
+
+	Ok(value)
+}
+
 #[cfg(test)]
 mod test {
 	use serde_json;