Clef: cliui mixes extraneous and approval-specific Data

[holiman]

Ref: NCC-EF-Clef-006

When Clef receives a request through its exposed API, metadata is displayed to the user in
charge of handling it. This metadata includes a variety of fields unrelated to what is being
signed like IP address, user-agent, origin, etc. There are 6 calls to showMetadata() within
signer/core/cliui.go that drive this functionality. Some of these fields can be trivially
tampered with and might provide a false understanding as users could rely too heavily on
them instead of the important fields.
The following ‘malicious’ request (with the redacted JSON taken from Go code example com-
ments) will be accepted by Clef.

curl-i http://localhost:8550/ \
-H "Content-Type: application/json"\ 
-X POST --data '{...}' \
-A "indicates INVALID CHECKSUM IS EXPECTED" \
-H "Origin: NCC Group requires IMMEDIATE APPROVAL per direction of J Smith"

As currently presented, the metadata provides little benefit to legitimate requests but may
facilitate illegitimate requests. A naive user may consider the extraneous request data as
superseding the true warning above and mistakenly approve this transaction.

Recommendation

Do not present request metadata alongside approval-specific data without clear delineation
and warnings. Either clearly label the categories presented and warn that request data
cannot be relied upon, or simply remove all request data.

---

Todo: look into if we can make this even more clear than it is currently.

[holiman]

I made a slight change, now it looks like this:

--------- Transaction request-------------
to:    0x07a565b7ed7d7a678680a4c162885bedbb695fe0

WARNING: Invalid checksum on to-address!

from:     0x82A2A876D39022B3019932D30Cd9c97ad5616813 [chksum ok]
value:    16 wei
gas:      0x333 (819)
gasprice: 291 wei
nonce:    0x0 (0)
data:     0x4401a6e40000000000000000000000000000000000000000000000000000000000000012

Transaction validation:
  * WARNING : Invalid checksum on to-address
  * Info : safeSend(address: 0x0000000000000000000000000000000000000012)


Request context:
	127.0.0.1:47808 -> HTTP/1.1 -> localhost:8551

Additional HTTP header data, provided by the external caller:
	User-Agent: indicates INVALID CHECKSUM IS EXPECTED
	Origin: NCC Group requires IMMEDIATE APPROVAL per direction of J Smith
-------------------------------------------
Approve? [y/N]:

I think that should be sufficient. We cannot remove all request data, in 99.9% of the cases, it's highly valuable information which can protect against drive-by-download types of attacks, where a webpage spuriously sends a request to the signer. By showing origin, the user would see what webpage is doing this. By showing user-agent, the user could see if it's his own browser or something else doing the requests -- assuming an attacker does not know what browser the user is using/

So overall, I think it's better to have it than to remove it.

[matias904]

Hohono