Skip to content

Commit 733e16c

Browse files
committed
decode: don't treat every failure as a rune.
If we can't decode something, and it decodes as a rune (and all bech32 strings do!), then we would usually just complain it was a malformed rune. Be a big more useful, when the parameter looks like somthing else. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Changelog-Fixed: JSON-RPC: `decode` is now more informative with malformed strings (won't claim everything is a malformed rune!).
1 parent b53d1bf commit 733e16c

File tree

6 files changed

+405
-22
lines changed

6 files changed

+405
-22
lines changed

common/utils.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,15 @@ char *str_lowering(const void *ctx, const char *string TAKES)
215215
return ret;
216216
}
217217

218+
char *str_uppering(const void *ctx, const char *string TAKES)
219+
{
220+
char *ret;
221+
222+
ret = tal_strdup(ctx, string);
223+
for (char *p = ret; *p; p++) *p = toupper(*p);
224+
return ret;
225+
}
226+
218227
/* Realloc helper for tal membufs */
219228
void *membuf_tal_resize(struct membuf *mb, void *rawelems, size_t newsize)
220229
{

common/utils.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,15 @@ void *membuf_tal_resize(struct membuf *mb, void *rawelems, size_t newsize);
188188
*/
189189
char *str_lowering(const void *ctx, const char *string TAKES);
190190

191+
/**
192+
* tal_struppering - return the same string by in upper case.
193+
* @ctx: the context to tal from (often NULL)
194+
* @string: the string that is going to be UPPERED (can be take())
195+
*
196+
* FIXME: move this in ccan
197+
*/
198+
char *str_uppering(const void *ctx, const char *string TAKES);
199+
191200
/**
192201
* Assign two different structs which are the same size.
193202
* We use this for assigning secrets <-> sha256 for example.

plugins/libplugin.h

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -416,14 +416,12 @@ command_success(struct command *cmd, const struct json_out *result)
416416
NON_NULL_ARGS(1, 2);
417417

418418
/* End a hook normally (with "result": "continue") */
419-
struct command_result *WARN_UNUSED_RESULT
420-
command_hook_success(struct command *cmd)
421-
NON_NULL_ARGS(1);
419+
struct command_result *command_hook_success(struct command *cmd)
420+
WARN_UNUSED_RESULT NON_NULL_ARGS(1);
422421

423422
/* End a notification handler. */
424-
struct command_result *WARN_UNUSED_RESULT
425-
notification_handled(struct command *cmd)
426-
NON_NULL_ARGS(1);
423+
struct command_result *notification_handled(struct command *cmd)
424+
WARN_UNUSED_RESULT NON_NULL_ARGS(1);
427425

428426
/* End a command created with aux_command. */
429427
struct command_result *WARN_UNUSED_RESULT

plugins/offers.c

Lines changed: 106 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
#include "config.h"
33
#include <ccan/array_size/array_size.h>
44
#include <ccan/bitops/bitops.h>
5+
#include <ccan/mem/mem.h>
56
#include <ccan/rune/rune.h>
67
#include <ccan/tal/str/str.h>
78
#include <common/bech32.h>
@@ -492,6 +493,91 @@ static u8 *encrypted_decode(const tal_t *ctx, const char *str, char **fail) {
492493
return NULL;
493494
}
494495

496+
enum likely_type {
497+
LIKELY_BOLT12_OFFER,
498+
LIKELY_BOLT12_INV,
499+
LIKELY_BOLT12_INVREQ,
500+
LIKELY_EMERGENCY_RECOVER,
501+
LIKELY_BOLT11,
502+
LIKELY_OTHER,
503+
};
504+
505+
/* Pull, either case! Advances tok->start on success. */
506+
static bool tok_pull(const char *buffer, jsmntok_t *tok, const char *lowerstr)
507+
{
508+
if (!json_tok_startswith(buffer, tok, lowerstr)) {
509+
const char *upperstr = str_uppering(tmpctx, lowerstr);
510+
if (!json_tok_startswith(buffer, tok, upperstr))
511+
return false;
512+
}
513+
tok->start += strlen(lowerstr);
514+
return true;
515+
}
516+
517+
static enum likely_type guess_type(const char *buffer, const jsmntok_t *tok)
518+
{
519+
jsmntok_t tok_copy = *tok;
520+
521+
if (tok_pull(buffer, &tok_copy, "lno1"))
522+
return LIKELY_BOLT12_OFFER;
523+
if (tok_pull(buffer, &tok_copy, "lni1"))
524+
return LIKELY_BOLT12_INV;
525+
if (tok_pull(buffer, &tok_copy, "lnr1"))
526+
return LIKELY_BOLT12_INVREQ;
527+
if (tok_pull(buffer, &tok_copy, "clnemerg1"))
528+
return LIKELY_EMERGENCY_RECOVER;
529+
/* BOLT #11:
530+
*
531+
* The human-readable part of a Lightning invoice consists of
532+
* two sections:
533+
*
534+
* 1. `prefix`: `ln` + BIP-0173 currency prefix (e.g. `lnbc`
535+
* for Bitcoin mainnet, `lntb` for Bitcoin testnet, `lntbs`
536+
* for Bitcoin signet, and `lnbcrt` for Bitcoin regtest)
537+
*
538+
* 1. `amount`: optional number in that currency, followed by
539+
* an optional `multiplier` letter. The unit encoded here
540+
* is the 'social' convention of a payment unit -- in the
541+
* case of Bitcoin the unit is 'bitcoin' NOT satoshis.
542+
*/
543+
if (tok_pull(buffer, &tok_copy, "lnbcrt")
544+
|| tok_pull(buffer, &tok_copy, "lnbc")
545+
|| tok_pull(buffer, &tok_copy, "lntbs")
546+
|| tok_pull(buffer, &tok_copy, "lntb")) {
547+
/* Now find last '1', which separates hrp from data */
548+
const char *delim = memrchr(buffer + tok_copy.start, '1',
549+
tok_copy.end - tok_copy.start);
550+
if (!delim)
551+
return LIKELY_OTHER;
552+
553+
/* BOLT #11:
554+
* The following `multiplier` letters are defined:
555+
*
556+
* * `m` (milli): multiply by 0.001
557+
* * `u` (micro): multiply by 0.000001
558+
* * `n` (nano): multiply by 0.000000001
559+
* * `p` (pico): multiply by 0.000000000001
560+
*/
561+
delim--;
562+
if (delim > buffer + tok_copy.start
563+
&& (tolower(*delim) == 'm'
564+
|| tolower(*delim) == 'u'
565+
|| tolower(*delim) == 'n'
566+
|| tolower(*delim) == 'p')) {
567+
delim--;
568+
}
569+
570+
while (delim >= buffer + tok_copy.start) {
571+
if (!cisdigit(*delim))
572+
return LIKELY_OTHER;
573+
delim--;
574+
}
575+
return LIKELY_BOLT11;
576+
}
577+
578+
return LIKELY_OTHER;
579+
}
580+
495581
static struct command_result *param_decodable(struct command *cmd,
496582
const char *name,
497583
const char *buffer,
@@ -500,21 +586,22 @@ static struct command_result *param_decodable(struct command *cmd,
500586
{
501587
char *likely_fail = NULL, *fail;
502588
jsmntok_t tok;
589+
enum likely_type type;
503590

504591
/* BOLT #11:
505592
*
506593
* If a URI scheme is desired, the current recommendation is to either
507594
* use 'lightning:' as a prefix before the BOLT-11 encoding
508595
*/
509596
tok = *token;
510-
if (json_tok_startswith(buffer, &tok, "lightning:")
511-
|| json_tok_startswith(buffer, &tok, "LIGHTNING:"))
512-
tok.start += strlen("lightning:");
597+
/* Note: either case! */
598+
tok_pull(buffer, &tok, "lightning:");
513599

600+
type = guess_type(buffer, &tok);
514601
decodable->offer = offer_decode(cmd, buffer + tok.start,
515602
tok.end - tok.start,
516603
plugin_feature_set(cmd->plugin), NULL,
517-
json_tok_startswith(buffer, &tok, "lno1")
604+
type == LIKELY_BOLT12_OFFER
518605
? &likely_fail : &fail);
519606
if (decodable->offer) {
520607
decodable->type = "bolt12 offer";
@@ -525,8 +612,7 @@ static struct command_result *param_decodable(struct command *cmd,
525612
tok.end - tok.start,
526613
plugin_feature_set(cmd->plugin),
527614
NULL,
528-
json_tok_startswith(buffer, &tok,
529-
"lni1")
615+
type == LIKELY_BOLT12_INV
530616
? &likely_fail : &fail);
531617
if (decodable->invoice) {
532618
decodable->type = "bolt12 invoice";
@@ -537,8 +623,7 @@ static struct command_result *param_decodable(struct command *cmd,
537623
tok.end - tok.start,
538624
plugin_feature_set(cmd->plugin),
539625
NULL,
540-
json_tok_startswith(buffer, &tok,
541-
"lnr1")
626+
type == LIKELY_BOLT12_INVREQ
542627
? &likely_fail : &fail);
543628
if (decodable->invreq) {
544629
decodable->type = "bolt12 invoice_request";
@@ -547,22 +632,20 @@ static struct command_result *param_decodable(struct command *cmd,
547632

548633
decodable->emergency_recover = encrypted_decode(cmd, tal_strndup(tmpctx, buffer + tok.start,
549634
tok.end - tok.start),
550-
json_tok_startswith(buffer, &tok,
551-
"clnemerg1")
635+
type == LIKELY_EMERGENCY_RECOVER
552636
? &likely_fail : &fail);
553637

554638
if (decodable->emergency_recover) {
555639
decodable->type = "emergency recover";
556640
return NULL;
557641
}
558642

559-
/* If no other was likely, bolt11 decoder gives us failure string. */
560643
decodable->b11 = bolt11_decode(cmd,
561644
tal_strndup(tmpctx, buffer + tok.start,
562645
tok.end - tok.start),
563646
plugin_feature_set(cmd->plugin),
564647
NULL, NULL,
565-
likely_fail ? &fail : &likely_fail);
648+
type == LIKELY_BOLT11 ? &likely_fail : &fail);
566649
if (decodable->b11) {
567650
decodable->type = "bolt11 invoice";
568651
return NULL;
@@ -571,10 +654,19 @@ static struct command_result *param_decodable(struct command *cmd,
571654
decodable->rune = rune_from_base64n(decodable, buffer + tok.start,
572655
tok.end - tok.start);
573656
if (decodable->rune) {
574-
decodable->type = "rune";
575-
return NULL;
657+
/* Any bech32 string will "parse" as a rune, but that's not
658+
* helpful. If it isn't all valid UTF8, and it looks like a
659+
* different type, reject it as that one. */
660+
const char *string = rune_to_string(tmpctx, decodable->rune);
661+
if (utf8_check(string, strlen(string)) || type == LIKELY_OTHER) {
662+
decodable->type = "rune";
663+
return NULL;
664+
}
576665
}
577666

667+
if (!likely_fail)
668+
likely_fail = "Unparsable string";
669+
578670
/* Return failure message from most likely parsing candidate */
579671
return command_fail_badparam(cmd, name, buffer, &tok, likely_fail);
580672
}

0 commit comments

Comments
 (0)