fix(bolt12): Add ASCII validation for offer currency field

[erickcestari]

After performing differential fuzzing using bitcoinfuzz with rust-lightning and Core Lightning (CLN), I noticed that rust-lightning does not verify whether the offer_currency field contains valid UTF-8.

This commit adds validation to ensure that offer_currency contains valid UTF-8 bytes, as required by the BOLT 12 specification.

[ldk-reviews-bot]

👋 Thanks for assigning @jkczyz as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

[codecov]

Codecov Report

Attention: Patch coverage is 93.93939% with 2 lines in your changes missing coverage. Please review.

Project coverage is 89.74%. Comparing base (78fee88) to head (91e633a).
Report is 73 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/offers/offer.rs	93.93%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3814      +/-   ##
==========================================
+ Coverage   89.52%   89.74%   +0.21%     
==========================================
  Files         157      159       +2     
  Lines      125100   128942    +3842     
  Branches   125100   128942    +3842     
==========================================
+ Hits       112002   115723    +3721     
- Misses      10408    10518     +110     
- Partials     2690     2701      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[erickcestari]

I took a closer look, and it seems this test vector has an invalid currency length in UTF-8 encoding, rather than being an invalid UTF-8 string itself.

  {
    "description": "Malformed: invalid currency UTF-8",
    "valid": false,
    "bolt12": "lno1qcpgqsg2q4q5cj2rg5tzzqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqg"
  },

data part: 060280410a05414c4943451621020202020202020202020202020202020202020202020202020202020202020202

Looking at the data part, we observe 06 (currency type), 02 (length), and 2804 (value), which confirms that the error arises from an invalid currency length in UTF-8 encoding, not from an invalid UTF-8 sequence.

I'll open a PR on Bolts to fix it.

[TheBlueMatt]

Aren't the currencies supposed to be the three-char currency specification standard? ie they should be ASCII only (and probably all-caps letter only, never numbers)?

[jkczyz]

Thanks for jumping on this!

[jkczyz]

I took a closer look, and it seems this test vector has an invalid currency length in UTF-8 encoding, rather than being an invalid UTF-8 string itself.

  {
    "description": "Malformed: invalid currency UTF-8",
    "valid": false,
    "bolt12": "lno1qcpgqsg2q4q5cj2rg5tzzqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqg"
  },

data part: 060280410a05414c4943451621020202020202020202020202020202020202020202020202020202020202020202

Looking at the data part, we observe 06 (currency type), 02 (length), and 2804 (value), which confirms that the error arises from an invalid currency length in UTF-8 encoding, not from an invalid UTF-8 sequence.

I'll open a PR on Bolts to fix it.

Ah, yeah either the test vector or the comment is wrong lol

[erickcestari]

Aren't the currencies supposed to be the three-char currency specification standard? ie they should be ASCII only (and probably all-caps letter only, never numbers)?

Initially, I considered just aligning it with the UTF-8 type for consistency. But yes, it makes more sense to enforce stricter validation to ensure it's valid ASCII and adheres to the expected format.

[ldk-reviews-bot]

🔔 1st Reminder

Hey @TheBlueMatt @jkczyz! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.