Skip to content

Commit

Permalink
Fixed signature malleability issue
Browse files Browse the repository at this point in the history
  • Loading branch information
StealthSend committed Nov 24, 2015
1 parent 85de000 commit e18175f
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 10 deletions.
1 change: 1 addition & 0 deletions src/checkpoints.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ namespace Checkpoints
( 100500, uint256("0x80749a6d5ed2ab45312c11206a355f81c9ec6e3effd2f835ec007caf40865c6f"))
( 105000, uint256("0xacc3cc5ecf67849d7616f9f0f6e955e7288be2b93a614e0be52b7bd540327071"))
( 347000, uint256("0x4aaa94b5b7018607a19301e7ba63d40cc3024f091c1bcffaf2b64ef0e1ac5bcb"))
( 769000, uint256("0x7424eff7b5800cfc59e1420c4c32611bceaebb34959a1df93ca678bb5c614582"))
;
;

Expand Down
4 changes: 2 additions & 2 deletions src/clientversion.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
// These need to be macros, as version.cpp's and bitcoin-qt.rc's voodoo requires it
#define CLIENT_VERSION_MAJOR 2
#define CLIENT_VERSION_MINOR 0
#define CLIENT_VERSION_REVISION 0
#define CLIENT_VERSION_BUILD 4
#define CLIENT_VERSION_REVISION 2
#define CLIENT_VERSION_BUILD 0

// Converts the parameter X to a string after macro replacement on X has been performed.
// Don't merge these into one macro!
Expand Down
69 changes: 65 additions & 4 deletions src/key.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -448,15 +448,76 @@ bool CKey::SetCompactSignature(uint256 hash, const std::vector<unsigned char>& v
return false;
}

bool CKey::Verify(uint256 hash, const std::vector<unsigned char>& vchSig)
// bool CKey::Verify(uint256 hash, const std::vector<unsigned char>& vchSig)
// {
// // -1 = error, 0 = bad sig, 1 = good
// if (ECDSA_verify(0, (unsigned char*)&hash, sizeof(hash), &vchSig[0], vchSig.size(), pkey) != 1)
// return false;
//
// return true;
// }


// Credit: https://github.com/ppcoin/ppcoin/pull/101/files
bool CKey::Verify(uint256 hash, const std::vector<unsigned char>& vchSigParam)
{
// -1 = error, 0 = bad sig, 1 = good
if (ECDSA_verify(0, (unsigned char*)&hash, sizeof(hash), &vchSig[0], vchSig.size(), pkey) != 1)
// Prevent the problem described here:
// https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2015-July/009697.html
// by removing the extra length bytes
std::vector<unsigned char> vchSig(vchSigParam.begin(), vchSigParam.end());
if (vchSig.size() > 1 && vchSig[1] & 0x80)
{
unsigned char nLengthBytes = vchSig[1] & 0x7f;

if (vchSig.size() < 2 + nLengthBytes)
return false;

if (nLengthBytes > 4)
{
unsigned char nExtraBytes = nLengthBytes - 4;
for (unsigned char i = 0; i < nExtraBytes; i++)
if (vchSig[2 + i])
return false;
vchSig.erase(vchSig.begin() + 2, vchSig.begin() + 2 + nExtraBytes);
vchSig[1] = 0x80 | (nLengthBytes - nExtraBytes);
}
}

if (vchSig.empty())
return false;

return true;
// New versions of OpenSSL will reject non-canonical DER signatures. de/re-serialize first.
unsigned char *norm_der = NULL;
ECDSA_SIG *norm_sig = ECDSA_SIG_new();
const unsigned char* sigptr = &vchSig[0];
assert(norm_sig);
if (d2i_ECDSA_SIG(&norm_sig, &sigptr, vchSig.size()) == NULL)
{
/* As of OpenSSL 1.0.0p d2i_ECDSA_SIG frees and nulls the pointer on
* error. But OpenSSL's own use of this function redundantly frees the
* result. As ECDSA_SIG_free(NULL) is a no-op, and in the absence of a
* clear contract for the function behaving the same way is more
* conservative.
*/
ECDSA_SIG_free(norm_sig);
return false;
}
int derlen = i2d_ECDSA_SIG(norm_sig, &norm_der);
ECDSA_SIG_free(norm_sig);
if (derlen <= 0)
return false;

// -1 = error, 0 = bad sig, 1 = good
bool ret = ECDSA_verify(0, (unsigned char*)&hash, sizeof(hash), norm_der, derlen, pkey) == 1;
OPENSSL_free(norm_der);
return ret;
}






bool CKey::VerifyCompact(uint256 hash, const std::vector<unsigned char>& vchSig)
{
CKey key;
Expand Down
2 changes: 1 addition & 1 deletion src/key.h
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ class CKey
// (the signature is a valid signature of the given data for that key)
bool SetCompactSignature(uint256 hash, const std::vector<unsigned char>& vchSig);

bool Verify(uint256 hash, const std::vector<unsigned char>& vchSig);
bool Verify(uint256 hash, const std::vector<unsigned char>& vchSigParam);

// Verify a compact signature
bool VerifyCompact(uint256 hash, const std::vector<unsigned char>& vchSig);
Expand Down
4 changes: 3 additions & 1 deletion src/makefile.debian
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@
# cd Installers/libcryptopp/cryptopp56
# wget http://www.cryptopp.com/cryptopp562.zip
# unzip -a cryptopp562.zip
# make
# make static
# make dynamic
# make all
# sudo make install
#
# Then, you can build
Expand Down
5 changes: 3 additions & 2 deletions src/version.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ extern const std::string CLIENT_DATE;
// Different keys for alerts and hash sync checkpoints
// 62010 : New rule to accept duplicate stake on bootstrap (only!)
// Technically not a network protocol difference
static const int PROTOCOL_VERSION = 62010;
// 62020 : Fixes signature malleability
static const int PROTOCOL_VERSION = 62020;

// earlier versions not supported as of Feb 2012, and are disconnected
static const int MIN_PROTO_VERSION = 61300;
Expand All @@ -53,7 +54,7 @@ static const int DATABASE_VERSION = 61201;

#define DISPLAY_VERSION_MAJOR 2
#define DISPLAY_VERSION_MINOR 0
#define DISPLAY_VERSION_REVISION 1
#define DISPLAY_VERSION_REVISION 2
#define DISPLAY_VERSION_BUILD 0

#endif

0 comments on commit e18175f

Please sign in to comment.