validatedSignatureChainHash added to withdrawSigned

[SibghatUllah1997]

New Feature: validatedSignatureChainHash
File/Module Affected: withdrawSigned functionality

Description:

A new feature, validatedSignatureChainHash, has been introduced.
Purpose: This enhancement is implemented to improve the security and integrity of the withdrawSigned function. It adds an additional layer of verification for node signatures.
Impact: Ensures that the signatures are verified more robustly, enhancing the overall security of the process.

[naiemk]

How does it add security on the SC level? This hash is now included as part of the message but the SC has no way of veryfing anything about it

[taha-abbasi]

How does it add security on the SC level? This hash is now included as part of the message, but the SC cannot verify anything about it.

Here is the idea @naiemk
By the time we get to masterNode, it has verified signatureChain, i.e., verified that the generator and validator set of signatures are valid. When it generates the withdrawSignature, it includes the following values before this change:

address token
address payee
uint256 amount
bytes32 salt
uint256 expiry

It uses these values and the provided masterNode signature to validate the withdrawRequest. However, in this scenario, nothing connects the signatureChain verified on masterNode to SC for withdrawal.

To connect the singatureChain validation from the node to the SC, we are now passing the validatedSignatureChainHash (signatureChain validated in encoded form) to the withdrawal request as a parameter.

What will this benefit? The idea is that when the SC verifies the withdrawSignature, it will fail if the signatureChain being passed has been tampered with. i.e., someone tries to pass a withdrawal request with a malicious generator or validator signature. Only the signatureChain validated by the masterNode can be verified.

[taha-abbasi]

@SibghatUllah1997 @zikriya I just had a conversation with @naiemk about the signatureChainhash and the need to include it on the withdrawal contract side vs the security benefit. There doesn't seem to be an enhanced security benefit in including this data on the SC side. The reason being that the master node has already validated this information, and if the hash is included or not in the SC (withdrawal request) it doesn't increase the security. Because if an attacker is able to compromise the authority signature they can simply add any hash data to the validatedSignature hash and have a valid withdrawal item.

The if we were to validated the signatureChain on chain then we'd need to add generator and Validators as signers and validate their signatures too which will significantly complicate the withdrawal item and increase gas. This is already being addressed when we integrate with QPN.

So for now, here are the next steps:

1. @SibghatUllah1997 rever the change on the withdrawal functions to remove validatedSignatureChainHash
2. @zikriya when generating the signature on masterNode. Make sure that the signature chain is validated, but don't include it in the data that is used to generate the withdrawal signature.

Resubmit the PR when changes are complete and tag @naiemk and myself in Slack please.