backend icon indicating copy to clipboard operation
backend copied to clipboard

Malformed BASIC_TRANSFER transactions not rejected

Open StarGeezerPhil opened this issue 1 year ago • 4 comments

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: image

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 avatar Jun 20 '24 20:06 StarGeezerPhil

Suggested change to basic_transer.go https://github.com/deso-protocol/backend/blob/e6eadebc934e18f6c3e6c595b03fa8bef6a40c7d/scripts/tools/toolslib/basic_transfer.go

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

StarGeezerPhil avatar Jun 20 '24 20:06 StarGeezerPhil

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

diamondhands0 avatar Jun 20 '24 20:06 diamondhands0

Pull request with validation: @diamondhands0

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

StarGeezerPhil avatar Jun 20 '24 20:06 StarGeezerPhil

@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?

diamondhands0 avatar Jun 21 '24 19:06 diamondhands0