smite-ir: add malformation parameter to funding_created - #235
Open
NishantBansal2003 wants to merge 4 commits into
Open
NishantBansal2003 wants to merge 4 commits into
NishantBansal2003 wants to merge 4 commits into
Conversation
Signed-off-by: Nishant Bansal <nishant.bansal.282003@gmail.com>
Some message fields are computed rather than supplied as IR variables, so no mutator can give them invalid values. Add Malformation to overwrite such a field in the encoded message before sending. MalformableField and MessageType::malformable_fields() live in bolt next to the codecs. Signed-off-by: Nishant Bansal <nishant.bansal.282003@gmail.com>
Add an optional Malformation to SendFundingCreated so the parameter mutator can overwrite funding_txid, funding_output_index or signature. It can set, replace or clear the malformation, using random bytes, a repeated byte or an interesting integer. FundingCreated now lists these fields and their offsets. The executor does not apply the malformation yet. Signed-off-by: Nishant Bansal <nishant.bansal.282003@gmail.com>
The executor now applies the malformation before building the commitment, so the channel it tracks matches what was sent. A malformed outpoint changes which channel is tracked and marks the outpoint invalid. A malformed signature is sent unchanged and marks the signature invalid, even if it no longer decodes. Signed-off-by: Nishant Bansal <nishant.bansal.282003@gmail.com>
NishantBansal2003
force-pushed
the
semantic-field-malform
branch
from
September 17, 2026 09:33
b62d223 to
a8fd818
Compare
NishantBansal2003
marked this pull request as ready for review
September 17, 2026 09:38
erickcestari
suggested changes
Sep 17, 2026
Comment on lines
+141
to
+158
| /// A message field whose encoded bytes can be overwritten before the message | ||
| /// is sent. | ||
| /// | ||
| /// These identify fields for which the IR can only construct valid values, so | ||
| /// invalid values can only be put on the wire by overwriting their encoded | ||
| /// bytes. All other fields are reachable through the IR's parameters. | ||
| /// | ||
| /// Offsets include the 2-byte message type prefix and therefore index directly | ||
| /// into the encoded message. | ||
| #[derive(Debug, Clone, Copy, PartialEq, Eq)] | ||
| pub struct MalformableField { | ||
| /// Field name to overwrite. | ||
| pub name: &'static str, | ||
| /// Byte offset in the encoded message, including the message type prefix. | ||
| pub offset: u16, | ||
| /// Field length in bytes. | ||
| pub len: u16, | ||
| } |
Contributor
There was a problem hiding this comment.
AFIK we haven't mentioned the IR in bolts crate until this PR. I think it would be better if bolts had no IR knowledge.
Suggested change
| /// A message field whose encoded bytes can be overwritten before the message | |
| /// is sent. | |
| /// | |
| /// These identify fields for which the IR can only construct valid values, so | |
| /// invalid values can only be put on the wire by overwriting their encoded | |
| /// bytes. All other fields are reachable through the IR's parameters. | |
| /// | |
| /// Offsets include the 2-byte message type prefix and therefore index directly | |
| /// into the encoded message. | |
| #[derive(Debug, Clone, Copy, PartialEq, Eq)] | |
| pub struct MalformableField { | |
| /// Field name to overwrite. | |
| pub name: &'static str, | |
| /// Byte offset in the encoded message, including the message type prefix. | |
| pub offset: u16, | |
| /// Field length in bytes. | |
| pub len: u16, | |
| } | |
| /// Location of a fixed-offset field within an encoded message. | |
| #[derive(Debug, Clone, Copy, PartialEq, Eq)] | |
| pub struct FieldSpan { | |
| /// Field name as written in the BOLT spec. | |
| pub name: &'static str, | |
| /// Byte offset in the encoded message, including the message type prefix. | |
| pub offset: u16, | |
| /// Field length in bytes. | |
| pub len: u16, | |
| } |
Comment on lines
+46
to
+50
| pub const MALFORMABLE_FIELDS: &'static [MalformableField] = &[ | ||
| Self::FUNDING_TXID_FIELD, | ||
| Self::FUNDING_OUTPUT_INDEX_FIELD, | ||
| Self::SIGNATURE_FIELD, | ||
| ]; |
Contributor
There was a problem hiding this comment.
This should live inside the malform.rs, since the fields that are unreachable depends on the IR's operation, not on the codecs.
Suggested change
| pub const MALFORMABLE_FIELDS: &'static [MalformableField] = &[ | |
| Self::FUNDING_TXID_FIELD, | |
| Self::FUNDING_OUTPUT_INDEX_FIELD, | |
| Self::SIGNATURE_FIELD, | |
| ]; |
| assert!(state.is_funding_outpoint_valid); | ||
| assert!(state.sent_invalid_signature); | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
These two returns aren't being covered in the tests if they're also applying correctly the malformation.
// Without both the recorded `open_channel` and the peer's `accept_channel`
// we cannot build the commitment to sign, so fall back to an unsigned
// `funding_created` and leave `channel_states` untouched.
let Some(pending) = negotiations.get(&temporary_channel_id) else {
return Ok(encoded);
};
let open_channel = &pending.open_channel;
let Some(accept_channel) = pending.accept_channel.as_ref() else {
return Ok(encoded);
};
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ref: #178
Depends-on: #212
Add a malformation parameter to
SendFundingCreatedthat overwrites an allowlisted field offunding_createdbefore sending it to the target. Some fields currently only contain valid values, so this allows the IR to exercise their invalid values as well.Oracle state is updated based on the message actually sent to the target, after applying the malformation. If the malformed signature cannot be decoded, we send only the invalid signature without marking the state as desynced, since the target should simply reject it as an invalid signature rather than behave differently.
The generator produces only non-malformed
funding_createdmessages,OperationParamMutatoradds malformations using repeated bytes, random bytes, or interesting integers matching the field's exact width.I verified it, and smite is now able to detect the bug disclosed in CLN
funding_txid==0assertion crash