From 62c49018edd95587e1358be3c5ec22cad1320407 Mon Sep 17 00:00:00 2001
From: Christoph Jentzsch <jentzsch.software@gmail.com>
Date: Mon, 17 Nov 2014 22:23:09 +0100
Subject: [PATCH] Check validity of signature values r,s,v

---
 libdevcore/FixedHash.h      |  1 +
 libdevcrypto/Common.cpp     |  9 +++++++++
 libdevcrypto/Common.h       | 10 +++++++++-
 libethereum/State.cpp       |  4 ++++
 libethereum/Transaction.cpp |  4 +++-
 5 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/libdevcore/FixedHash.h b/libdevcore/FixedHash.h
index 0e387ab8a..6c42aa501 100644
--- a/libdevcore/FixedHash.h
+++ b/libdevcore/FixedHash.h
@@ -83,6 +83,7 @@ public:
 	bool operator==(FixedHash const& _c) const { return m_data == _c.m_data; }
 	bool operator!=(FixedHash const& _c) const { return m_data != _c.m_data; }
 	bool operator<(FixedHash const& _c) const { return m_data < _c.m_data; }
+	bool operator>=(FixedHash const& _c) const { return m_data >= _c.m_data; }
 
 	// The obvious binary operators.
 	FixedHash& operator^=(FixedHash const& _c) { for (unsigned i = 0; i < N; ++i) m_data[i] ^= _c.m_data[i]; return *this; }
diff --git a/libdevcrypto/Common.cpp b/libdevcrypto/Common.cpp
index fa5a544a1..f4803bd52 100644
--- a/libdevcrypto/Common.cpp
+++ b/libdevcrypto/Common.cpp
@@ -33,6 +33,15 @@ using namespace dev::crypto;
 
 static Secp256k1 s_secp256k1;
 
+bool dev::SignatureStruct::isValid()
+{
+	if (this->v > 1 ||
+			this->r >= h256("0xfffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364141") ||
+			this->s >= h256("0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc2f"))
+		return false;
+	return true;
+}
+
 Public dev::toPublic(Secret const& _secret)
 {
 	Public p;
diff --git a/libdevcrypto/Common.h b/libdevcrypto/Common.h
index 2ec332d8d..96631fcf9 100644
--- a/libdevcrypto/Common.h
+++ b/libdevcrypto/Common.h
@@ -43,7 +43,15 @@ using Public = h512;
 /// @NOTE This is not endian-specific; it's just a bunch of bytes.
 using Signature = h520;
 
-struct SignatureStruct { h256 r; h256 s; byte v; };
+struct SignatureStruct
+{
+	/// @returns true if r,s,v values are valid, otherwise false
+	bool isValid();
+
+	h256 r;
+	h256 s;
+	byte v;
+};
 
 /// An Ethereum address: 20 bytes.
 /// @NOTE This is not endian-specific; it's just a bunch of bytes.
diff --git a/libethereum/State.cpp b/libethereum/State.cpp
index ee93fdc55..f657c2699 100644
--- a/libethereum/State.cpp
+++ b/libethereum/State.cpp
@@ -55,6 +55,10 @@ void ecrecoverCode(bytesConstRef _in, bytesRef _out)
 
 	memcpy(&in, _in.data(), min(_in.size(), sizeof(in)));
 
+	SignatureStruct sig{in.r, in.s, (byte)((int)(u256)in.v - 27)};
+	if (!sig.isValid() || in.v > 28)
+		return;
+
 	byte pubkey[65];
 	int pubkeylen = 65;
 	secp256k1_start();
diff --git a/libethereum/Transaction.cpp b/libethereum/Transaction.cpp
index d94a31425..1edfe3927 100644
--- a/libethereum/Transaction.cpp
+++ b/libethereum/Transaction.cpp
@@ -82,7 +82,9 @@ Address Transaction::sender() const
 void Transaction::sign(Secret _priv)
 {
 	auto sig = dev::sign(_priv, sha3(WithoutSignature));
-	m_vrs = *(SignatureStruct const*)&sig;
+	SignatureStruct sigStruct = *(SignatureStruct const*)&sig;
+	if (sigStruct.isValid())
+		m_vrs = sigStruct;
 }
 
 void Transaction::streamRLP(RLPStream& _s, IncludeSignature _sig) const