Ban unicode control characters from utf8 fields#1341
Conversation
morehouse
left a comment
morehouse
left a comment
There was a problem hiding this comment.
Does it also make sense to call out that the receiving node MAY reject gossip with non-conforming alias or features fields?
Eclair and LND in particular may want to reject such gossip during decoding since they currently don't preserve the underlying bytes of some non-conforming fields for signature verification (ACINQ/eclair#3314, lightningnetwork/lnd#10835).
|
Good idea, I added explicit requirements on the receiving side in f160681 |
tnull
left a comment
tnull
left a comment
There was a problem hiding this comment.
I do wonder if it would be worth to provide test vectors for this? Maybe they could be generated by reusing the script we recently added to LDK, see https://github.com/lightningdevkit/rust-lightning/pull/4605/changes#diff-af90f55a5f5e4af60b13fc942aee60e49311cb223f15bccdf373117b4aabc3df
As discussed in lightning#1260, we should ban NULL and other control characters in utf8 fields, along with characters that don't provide any value in node aliases: there's no valid use-case for them. Unfortunately, unicode cannot provide static lists of characters in each category, since it is open to extension. We thus more strongly restrict writers, and set reader restrictions that are more stable. We also make sure that feature bits MUST be minimally-encoded everywhere instead of being lenient for `node_announcement` for no good reason.
t-bast
force-pushed
the
disallow-null-chars
branch
from
f160681 to
dcd957d
Compare
4 participants
As discussed in #1260, we should ban NULL and other control characters in utf8 fields: there's no valid use-case for them.
We also make sure that feature bits MUST be minimally-encoded everywhere instead of being lenient for
node_announcementfor no good reason.