Adding functionality to test pubkey encoding, fails the script with appropriate error if the pubkey is not encoded correctly

src/main/scala/org/scalacoin/crypto/DERSignatureUtil.scala

@@ -24,7 +24,9 @@ trait DERSignatureUtil extends BitcoinSLogger {
    */
   def isDEREncoded(bytes : Seq[Byte]) : Boolean = {
     //signature is trivially valid if the signature is empty
-    if (!bytes.isEmpty) {
+
+    if (!bytes.isEmpty && bytes.size < 9) false
+    else if (!bytes.isEmpty) {
       //first byte must be 0x30
       val firstByteIs0x30 = bytes.head == 0x30
       logger.debug("firstByteIs0x30: " + firstByteIs0x30)

src/main/scala/org/scalacoin/crypto/TransactionSignatureChecker.scala

@@ -6,7 +6,7 @@ import org.scalacoin.protocol.transaction.{Transaction, TransactionInput}
 import org.scalacoin.script.{ScriptProgram}
 import org.scalacoin.script.crypto._
 import org.scalacoin.script.flag.{ScriptFlagUtil, ScriptFlag, ScriptVerifyDerSig}
-import org.scalacoin.util.{BitcoinSLogger, BitcoinSUtil}
+import org.scalacoin.util.{BitcoinScriptUtil, BitcoinSLogger, BitcoinSUtil}
 import org.slf4j.LoggerFactory
 
 import scala.annotation.tailrec
@@ -28,9 +28,13 @@ trait TransactionSignatureChecker extends BitcoinSLogger {
    */
   def checkSignature(txSignatureComponent : TransactionSignatureComponent,
                      pubKey: ECPublicKey, signature : ECDigitalSignature, flags : Seq[ScriptFlag]) : TransactionSignatureCheckerResult = {
+    val pubKeyEncodedCorrectly = BitcoinScriptUtil.checkPubKeyEncoding(pubKey,flags)
     if (ScriptFlagUtil.requiresStrictDerEncoding(flags) && !DERSignatureUtil.isStrictDEREncoding(signature)) {
-      logger.warn("Signature was not stricly encoded der: " + signature.hex)
+      logger.error("Signature was not stricly encoded der: " + signature.hex)
       SignatureValidationFailureNotStrictDerEncoding
+    } else if (!pubKeyEncodedCorrectly) {
+      logger.error("The public key given for signature checking was not encoded correctly")
+      SignatureValidationFailurePubKeyEncoding
     } else {
       //we need to check if the scriptSignature has a redeemScript
       //in that case, we need to pass the redeemScript to the TransactionSignatureChecker
@@ -50,7 +54,6 @@ trait TransactionSignatureChecker extends BitcoinSLogger {
       val isValid = pubKey.verify(hashForSignature,signature)
       if (isValid) SignatureValidationSuccess else SignatureValidationFailureIncorrectSignatures
     }
-
   }
 
   /**
@@ -95,6 +98,8 @@ trait TransactionSignatureChecker extends BitcoinSLogger {
           SignatureValidationFailureNotStrictDerEncoding
         case SignatureValidationFailureSignatureCount =>
           SignatureValidationFailureSignatureCount
+        case SignatureValidationFailurePubKeyEncoding =>
+          SignatureValidationFailurePubKeyEncoding
       }
     } else if (sigs.isEmpty) {
       //means that we have checked all of the sigs against the public keys

src/main/scala/org/scalacoin/crypto/TransactionSignatureCheckerResult.scala

@@ -46,3 +46,11 @@ case object SignatureValidationFailureIncorrectSignatures extends TransactionSig
 case object SignatureValidationFailureSignatureCount extends TransactionSignatureCheckerResult {
   def isValid = false
 }
+
+/**
+ * This indicates that the public key was not encoded correctly according to this function
+ * https://github.com/bitcoin/bitcoin/blob/master/src/script/interpreter.cpp#L202
+ */
+case object SignatureValidationFailurePubKeyEncoding extends TransactionSignatureCheckerResult {
+  def isValid = false
+}

src/main/scala/org/scalacoin/script/crypto/CryptoInterpreter.scala

@@ -95,8 +95,6 @@ trait CryptoInterpreter extends ControlOperationsInterpreter with BitcoinSLogger
         ScriptProgram(program,ScriptErrorSigDer)
       } else {
         val restOfStack = program.stack.tail.tail
-
-
         val result = TransactionSignatureChecker.checkSignature(program.txSignatureComponent,pubKey,
           signature,program.flags)
         logger.debug("signature verification isValid: " + result)
@@ -109,6 +107,9 @@ trait CryptoInterpreter extends ControlOperationsInterpreter with BitcoinSLogger
             ScriptProgram(program, ScriptFalse :: restOfStack,program.script.tail)
           case SignatureValidationFailureSignatureCount =>
             ScriptProgram(program, ScriptFalse :: restOfStack,program.script.tail)
+          case SignatureValidationFailurePubKeyEncoding =>
+            //means that a public key was not encoded correctly
+            ScriptProgram(program,ScriptErrorPubKeyType)
         }
       }
     }
@@ -177,7 +178,7 @@ trait CryptoInterpreter extends ControlOperationsInterpreter with BitcoinSLogger
           (program.stack.tail.slice(0,nPossibleSignatures.num.toInt),
             program.stack.tail.slice(nPossibleSignatures.num.toInt,program.stack.tail.size))
 
-        val pubKeys = pubKeysScriptTokens.map(key => ECFactory.publicKey(key.hex))
+        val pubKeys = pubKeysScriptTokens.map(key => ECFactory.publicKey(key.bytes))
         logger.debug("Public keys on the stack: " + pubKeys)
 
         logger.debug("mRequiredSignatures: " + mRequiredSignatures)
@@ -224,6 +225,9 @@ trait CryptoInterpreter extends ControlOperationsInterpreter with BitcoinSLogger
             case SignatureValidationFailureSignatureCount =>
               //means that we did not have enough signatures for OP_CHECKMULTISIG
               ScriptProgram(program, ScriptErrorSigCount)
+            case SignatureValidationFailurePubKeyEncoding =>
+              //means that a public key was not encoded correctly
+              ScriptProgram(program,ScriptErrorPubKeyType)
           }
         }
       }

src/main/scala/org/scalacoin/script/flag/ScriptFlagUtil.scala

@@ -15,6 +15,13 @@ trait ScriptFlagUtil {
     flags.contains(ScriptVerifyDerSig) || flags.contains(ScriptVerifyStrictEnc)
   }
 
+  /**
+   * Checks if we are required to check for strict encoding
+   * @param flags
+   * @return
+   */
+  def requireStrictEncoding(flags : Seq[ScriptFlag]) : Boolean = flags.contains(ScriptVerifyStrictEnc)
+
   /**
    * Checks if the script flag for checklocktimeverify is enabled
    * @param flags

src/main/scala/org/scalacoin/util/BitcoinScriptUtil.scala

@@ -1,6 +1,7 @@
 package org.scalacoin.util
 
-import org.scalacoin.script.flag.ScriptFlagUtil
+import org.scalacoin.crypto.ECPublicKey
+import org.scalacoin.script.flag.{ScriptFlag, ScriptFlagUtil}
 import org.scalacoin.script.{ScriptProgram, ExecutionInProgressScriptProgram, ScriptSettings}
 import org.scalacoin.script.constant._
 import org.scalacoin.script.crypto.{OP_CHECKMULTISIGVERIFY, OP_CHECKMULTISIG, OP_CHECKSIG, OP_CHECKSIGVERIFY}
@@ -190,9 +191,57 @@ trait BitcoinScriptUtil {
         false
       } else true
      } else true
-
   }
 
+  /**
+   * Checks the public key encoding according to bitcoin core's function
+   * https://github.com/bitcoin/bitcoin/blob/master/src/script/interpreter.cpp#L202
+   * @param key the key whose encoding we are checking
+   * @param program the program whose flags which dictate the rules for the public keys encoding
+   * @return if the key is encoded correctly against the rules give in the flags parameter
+   */
+  def checkPubKeyEncoding(key : ECPublicKey, program : ScriptProgram) : Boolean = checkPubKeyEncoding(key,program.flags)
+
+  /**
+   * Checks the public key encoding according to bitcoin core's function
+   * https://github.com/bitcoin/bitcoin/blob/master/src/script/interpreter.cpp#L202
+   * @param key the key whose encoding we are checking
+   * @param flags the flags which dictate the rules for the public keys encoding
+   * @return if the key is encoded correctly against the rules givein the flags parameter
+   */
+  def checkPubKeyEncoding(key : ECPublicKey, flags : Seq[ScriptFlag]) : Boolean = {
+    if (ScriptFlagUtil.requireStrictEncoding(flags) &&
+      !isCompressedOrUncompressedPubKey(key)) false else true
+  }
+
+
+  /**
+   * Returns true if the key is compressed or uncompressed, false otherwise
+   * https://github.com/bitcoin/bitcoin/blob/master/src/script/interpreter.cpp#L66
+   * @param key the public key that is being checked
+   * @return true if the key is compressed/uncompressed otherwise false
+   */
+  def isCompressedOrUncompressedPubKey(key : ECPublicKey) : Boolean = {
+    if (key.bytes.size < 33) {
+      //  Non-canonical public key: too short
+      return false
+    }
+    if (key.bytes.head == 0x04) {
+      if (key.bytes.size != 65) {
+        //  Non-canonical public key: invalid length for uncompressed key
+        return false
+      }
+    } else if (key.bytes.head == 0x02 || key.bytes.head == 0x03) {
+      if (key.bytes.size != 33) {
+        //  Non-canonical public key: invalid length for compressed key
+        return false
+      }
+    } else {
+      //  Non-canonical public key: neither compressed nor uncompressed
+      return false
+    }
+    return true
+  }
 }
 
 

src/test/scala/org/scalacoin/util/BitcoinScriptUtilTest.scala

@@ -1,5 +1,6 @@
 package org.scalacoin.util
 
+import org.scalacoin.crypto.ECFactory
 import org.scalacoin.script.bitwise.OP_EQUALVERIFY
 import org.scalacoin.script.constant._
 import org.scalacoin.script.crypto._
@@ -170,4 +171,11 @@ class BitcoinScriptUtilTest extends FlatSpec with MustMatchers {
     BitcoinScriptUtil.isShortestEncoding(ScriptConstantFactory.fromHex("ffff7f80")) must be (false)
     BitcoinScriptUtil.isShortestEncoding(ScriptConstantFactory.fromHex("ffff7f00")) must be (false)
   }
+
+  it must "check a public key's encoding" in {
+    //pubkeys must be compressed or uncompressed or else that are not validly encoded
+    val key = ECFactory.publicKey("00")
+    val program = TestUtil.testProgram
+    BitcoinScriptUtil.checkPubKeyEncoding(key,program) must be (false)
+  }
 }