Bad arguments (in C++ code) to BlindTransaction() can cause memory corruption

[dgpv]

elements/src/blind.cpp

Lines 407 to 416 in 4e023af

	if ((nPseudo == 0 && issuance_blinding_privkey.size() > nIn && issuance_blinding_privkey[nIn].IsValid()) ||
	(nPseudo == 1 && token_blinding_privkey.size() > nIn && token_blinding_privkey[nIn].IsValid())) {
	num_blind_attempts++;
	num_issuance_blind_attempts++;
	CAssetIssuance& issuance = tx.vin[nIn].assetIssuance;
	// First iteration does issuance asset, second inflation keys
	CConfidentialValue& conf_value = nPseudo ? issuance.nInflationKeys : issuance.nAmount;
	if (conf_value.IsNull()) {
	continue;
	}

If the issuance blinding key or token blinding key is specified for the input when calling BlindTransaction, but the transaction input itself have null value in issuance.nInflationKeys/issuance.nAmount, then num_blind_attempts will be incremented, while the size of value_blindptrs and asset_blindptrs will not be increased with another 32 bytes of random data.

(the same issue can happen with continue at

elements/src/blind.cpp

Lines 434 to 435 in 4e023af

	// Re-issuance only has one pseudo-input maximum
	continue;

)

If num_blind_attempts is larger than the size *_blindptrs arrays, secp256k1_pedersen_blind_generator_blind_sum that is called later can go over the size of the arrays, because there's no check that the sizes are equal to num_blind_attempts + num_known_input_blinds. This can cause crash. secp256k1_pedersen_blind_generator_blind_sum will also write to the last blinder, and this can cause memory corruption.

Of course this is only possible by the calling C++ code being incorrect, supplying wrong transaction data, etc. But this raises the question - are these continue statements actually needed ? Wouldn't it be better to replace them with returning failure ? It seems that in both cases, either the transaction structure is wrong, or the arguments issuance_blinding_privkey/token_blinding_privkey are wrong.