Malformed BASIC_TRANSFER transactions not rejected

StarGeezerPhil commented 5 months ago

It appears that malformed BASIC_TRANSFER transactions that do not include AmountNanos are not rejected by backend and can be signed and submitted into the mempool.

However, these transactions do not appear to be then accepted and mined into the chain.

@lazynina @diamondhands0 my concern is that this leaves us open to a vulnerability where the mempool could become hijacked in a DOS-type attack/clog the mempool entirely - that could overwhelm nodes and validators.

Here are the details - transactions created by user DiamondThumb:

This is his example payload provided:

Example transactions crash the blockchain explorer: https://explorer.deso.com/txn/3JuEUQSXue97tHWVrRidZC1GUU1Qdau78YskxHGukxByz1g6xxNCCo https://explorer.deso.com/txn/3JuETYrJjmN8LV1tvCfcZdsvMpNzB8pjzhQzAJ2BsojgcQJic4QkFu https://explorer.deso.com/txn/3JuETB3wvCm1UQ8bFNU7uRmoWZ9hE621e4waJDYzNDeYs5a9kfb41x https://explorer.deso.com/txn/3JuETt6hMbiYtrkAFUb1pVqWhwLj1xroXdmWbYmZzpY1SN7WKsFZua

Here is the verbose transaction details for the first example from my front-end:

{
  "TransactionIDBase58Check": "3JuEUQSXue97tHWVrRidZC1GUU1Qdau78YskxHGukxByz1g6xxNCCo",
  "TransactionHashHex": "db7fceaebbcedecbd933dfcfcc7d65d104ebc7d2578479313edeed5176716096",
  "RawTransactionHex": "00010360d99b827779f9e34fefbad4f23927fdf9bed8c90f820b0a374f7cdf03c98c5b00020021035a8155c505cc022a7bafc29d6a46ec6430cfeb68fe8cc16e613d04c6a1169d1f000001ab01ba8e15f5eb90a8aad2a0a652",
  "Outputs": [
    {
      "PublicKeyBase58Check": "BC1YLiMGYzDWEo73gJpqFZ32txELPQti7w13ccm9GTMyePHWtPNfxES",
      "AmountNanos": 0
    }
  ],
  "TransactionType": "BASIC_TRANSFER",
  "TransactionMetadata": {
    "BlockHashHex": "",
    "TxnIndexInBlock": 0,
    "TxnType": "BASIC_TRANSFER",
    "TransactorPublicKeyBase58Check": "BC1YLiJUUwY9Q5cbT1XCSVTp53piLWM3o1uCWvU8wAvG1wUVvNbupij",
    "AffectedPublicKeys": [
      {
        "PublicKeyBase58Check": "BC1YLiMGYzDWEo73gJpqFZ32txELPQti7w13ccm9GTMyePHWtPNfxES",
        "Metadata": "BasicTransferOutput"
      },
      {
        "PublicKeyBase58Check": "BC1YLiJUUwY9Q5cbT1XCSVTp53piLWM3o1uCWvU8wAvG1wUVvNbupij",
        "Metadata": "TransactorPublicKeyBase58Check"
      }
    ],
    "TxnOutputs": [
      {
        "PublicKey": "A2DZm4J3efnjT++61PI5J/35vtjJD4ILCjdPfN8DyYxb",
        "AmountNanos": 0
      }
    ],
    "BasicTransferTxindexMetadata": {
      "TotalInputNanos": 171,
      "TotalOutputNanos": 0,
      "FeeNanos": 171,
      "UtxoOpsDump": "",
      "UtxoOps": null,
      "DiamondLevel": 0,
      "PostHashHex": ""
    }
  },
  "TxnNonce": {
    "ExpirationBlockHeight": 345914,
    "PartialID": 5930258375685453000
  },
  "TxnFeeNanos": 171,
  "TxnVersion": 1,
  "RawTxnMetadata": {}
}

DiamondThumb's thread regarding his issue here: https://desocialworld.com/posts/20891c713fe091a02b4bd556b4f2cfeef4a758ec6fdf5a7f3522317d6557ee9e

StarGeezerPhil commented 5 months ago

func _generateUnsignedSendDeSo(senderPubKey *btcec.PublicKey, recipientPubKey *btcec.PublicKey, amountNanos int64,
    params *lib.DeSoParams, node string) (*routes.SendDeSoResponse, error) {
    **if !amountNanos || amountNanos <= 0 {
        return nil, errors.New("_generateUnsignedSendDeSo() invalid AmountNanos: must be a positive integer")
    }**
    endpoint := node + routes.RoutePathSendDeSo

diamondhands0 commented 5 months ago

Very interesting. We're taking a look at this now.

StarGeezerPhil commented 5 months ago

Pull request with validation: @diamondhands0

https://github.com/deso-protocol/backend/pull/646

diamondhands0 commented 5 months ago

@StarGeezerPhil this should be fixed now. It looks like the mempool was failing to reject txns with an invalid signature (the amount being zero was actually a red herring). Can you try submitting the txns again to node.deso.org and lmk your results?

deso-protocol / backend

Malformed BASIC_TRANSFER transactions not rejected #645