From 0e37c2dfc8b56e698d68910920e65ca02d1420cc Mon Sep 17 00:00:00 2001 From: Matt Whitlock Date: Mon, 14 Jul 2025 06:49:25 -0400 Subject: [PATCH 1/3] ccan: update to get json_escape_unescape_len() See: https://github.com/rustyrussell/ccan/pull/123 --- ccan/ccan/json_escape/json_escape.c | 40 +++++++++++++++++++++++------ ccan/ccan/json_escape/json_escape.h | 4 +++ 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/ccan/ccan/json_escape/json_escape.c b/ccan/ccan/json_escape/json_escape.c index daa14abe8c89..4bbb0032d1b5 100644 --- a/ccan/ccan/json_escape/json_escape.c +++ b/ccan/ccan/json_escape/json_escape.c @@ -1,6 +1,7 @@ /* MIT (BSD) license - see LICENSE file for details */ #include #include +#include struct json_escape *json_escape_string_(const tal_t *ctx, const void *bytes, size_t len) @@ -137,19 +138,24 @@ struct json_escape *json_escape_len(const tal_t *ctx, const char *str TAKES, } /* By policy, we don't handle \u. Use UTF-8. */ -const char *json_escape_unescape(const tal_t *ctx, const struct json_escape *esc) +static const char *unescape(const tal_t *ctx, const char *esc TAKES, size_t len) { - char *unesc = tal_arr(ctx, char, strlen(esc->s) + 1); + /* Fast path: can steal, and nothing to unescape. */ + if (is_taken(esc) && !memchr(esc, '\\', len)) + return tal_strndup(ctx, esc, len); + + char *unesc = tal_arr(ctx, char, len + 1); size_t i, n; - for (i = n = 0; esc->s[i]; i++, n++) { - if (esc->s[i] != '\\') { - unesc[n] = esc->s[i]; + for (i = n = 0; i < len; i++, n++) { + if (esc[i] != '\\') { + unesc[n] = esc[i]; continue; } - i++; - switch (esc->s[i]) { + if (++i == len) + goto error; + switch (esc[i]) { case 'n': unesc[n] = '\n'; break; @@ -168,13 +174,31 @@ const char *json_escape_unescape(const tal_t *ctx, const struct json_escape *esc case '/': case '\\': case '"': - unesc[n] = esc->s[i]; + unesc[n] = esc[i]; break; default: + error: + if (taken(esc)) + tal_free(esc); return tal_free(unesc); } } unesc[n] = '\0'; + if (!tal_resize(&unesc, n + 1)) + goto error; + if (taken(esc)) + tal_free(esc); return unesc; } + +const char *json_escape_unescape(const tal_t *ctx, const struct json_escape *esc) +{ + return unescape(ctx, esc->s, strlen(esc->s)); +} + +const char *json_escape_unescape_len(const tal_t *ctx, + const char *esc TAKES, size_t len) +{ + return unescape(ctx, esc, len); +} diff --git a/ccan/ccan/json_escape/json_escape.h b/ccan/ccan/json_escape/json_escape.h index 5b33432f8fce..88d1eefc1a64 100644 --- a/ccan/ccan/json_escape/json_escape.h +++ b/ccan/ccan/json_escape/json_escape.h @@ -41,4 +41,8 @@ struct json_escape *json_escape_string_(const tal_t *ctx, /* Be very careful here! Can fail! Doesn't handle \u: use UTF-8 please. */ const char *json_escape_unescape(const tal_t *ctx, const struct json_escape *esc); + +/* Be very careful here! Can fail! Doesn't handle \u: use UTF-8 please. */ +const char *json_escape_unescape_len(const tal_t *ctx, + const char *esc TAKES, size_t len); #endif /* CCAN_JSON_ESCAPE_H */ From 1361ddf2b1ec14ca1765f2a471361d76583dde14 Mon Sep 17 00:00:00 2001 From: Matt Whitlock Date: Mon, 14 Jul 2025 06:55:45 -0400 Subject: [PATCH 2/3] use json_escape_unescape_len() This avoids making an extra copy of the escaped string. Note that jsonrpc_command_add() no longer accepts usage strings containing invalid escape sequences. (Previously, it would quietly accept such a string without unescaping anything.) Changelog-None --- common/json_param.c | 6 ++---- common/test/run-bolt12_decode.c | 6 ++---- lightningd/jsonrpc.c | 10 ++-------- lightningd/runes.c | 6 ++---- 4 files changed, 8 insertions(+), 20 deletions(-) diff --git a/common/json_param.c b/common/json_param.c index b95952dd48be..1a761112cc38 100644 --- a/common/json_param.c +++ b/common/json_param.c @@ -444,11 +444,9 @@ struct command_result *param_escaped_string(struct command *cmd, const char **str) { if (tok->type == JSMN_STRING) { - struct json_escape *esc; /* jsmn always gives us ~ well-formed strings. */ - esc = json_escape_string_(cmd, buffer + tok->start, - tok->end - tok->start); - *str = json_escape_unescape(cmd, esc); + *str = json_escape_unescape_len(cmd, buffer + tok->start, + tok->end - tok->start); if (*str) return NULL; } diff --git a/common/test/run-bolt12_decode.c b/common/test/run-bolt12_decode.c index 0faa8ccef1fb..a6e896386f30 100644 --- a/common/test/run-bolt12_decode.c +++ b/common/test/run-bolt12_decode.c @@ -190,13 +190,11 @@ int main(int argc, char *argv[]) char *fail; const char *str; size_t dlen; - struct json_escape *esc; assert(json_to_bool(json, json_get_member(json, t, "valid"), &valid)); strtok = json_get_member(json, t, "string"); - esc = json_escape_string_(tmpctx, json + strtok->start, - strtok->end - strtok->start); - str = json_escape_unescape(tmpctx, esc); + str = json_escape_unescape_len(tmpctx, json + strtok->start, + strtok->end - strtok->start); actual = (string_to_data(tmpctx, str, strlen(str), "lno", &dlen, &fail) != NULL); assert(actual == valid); diff --git a/lightningd/jsonrpc.c b/lightningd/jsonrpc.c index 4b240808c83c..43286a289097 100644 --- a/lightningd/jsonrpc.c +++ b/lightningd/jsonrpc.c @@ -1393,20 +1393,14 @@ static void setup_command_usage(struct lightningd *ld, bool jsonrpc_command_add(struct jsonrpc *rpc, struct json_command *command, const char *usage TAKES) { - struct json_escape *esc; const char *unescaped; if (!command_add(rpc, command)) return false; - esc = json_escape_string_(tmpctx, usage, strlen(usage)); - unescaped = json_escape_unescape(command, esc); + unescaped = json_escape_unescape_len(command, usage, strlen(usage)); if (!unescaped) - unescaped = tal_strdup(command, usage); - else { - if (taken(usage)) - tal_free(usage); - } + return false; strmap_add(&rpc->usagemap, command->name, unescaped); tal_add_destructor2(command, destroy_json_command, rpc); diff --git a/lightningd/runes.c b/lightningd/runes.c index 9a4c6fdef859..77be7a30a0ad 100644 --- a/lightningd/runes.c +++ b/lightningd/runes.c @@ -484,10 +484,8 @@ static struct rune_altern *rune_altern_from_json(const tal_t *ctx, /* We still need to unescape here, for \\ -> \. JSON doesn't * allow unnecessary \ */ const char *unescape; - struct json_escape *e = json_escape_string_(tmpctx, - buffer + tok->start, - tok->end - tok->start); - unescape = json_escape_unescape(tmpctx, e); + unescape = json_escape_unescape_len(tmpctx, buffer + tok->start, + tok->end - tok->start); if (!unescape) return NULL; From 094bdef8093bf227b282cb0eb85d1422081a2617 Mon Sep 17 00:00:00 2001 From: Matt Whitlock Date: Mon, 14 Jul 2025 07:28:36 -0400 Subject: [PATCH 3/3] lightningd: unescape log message strings received from plugins Plugins can send arbitrary strings in "log" notifications. Lightningd was logging these strings without unescaping them, leading to ugly backslashes in the log. Unescape the strings before logging them. Changelog-Fixed: Log messages from plugins no longer contain an extra level of escaping. --- lightningd/plugin.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/lightningd/plugin.c b/lightningd/plugin.c index 7623476bd30a..257b40cbb654 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include @@ -487,6 +488,7 @@ static const char *plugin_log_handle(struct plugin *plugin, { const jsmntok_t *msgtok, *leveltok; enum log_level level; + const char *msg; bool call_notifier; msgtok = json_get_member(plugin->buffer, paramstok, "message"); leveltok = json_get_member(plugin->buffer, paramstok, "level"); @@ -511,10 +513,15 @@ static const char *plugin_log_handle(struct plugin *plugin, json_tok_full(plugin->buffer, leveltok)); } + msg = json_escape_unescape_len(tmpctx, plugin->buffer + msgtok->start, + msgtok->end - msgtok->start); + if (!msg) + return tal_fmt(plugin, "Log notification from plugin has a \"message\" " + "string containing an invalid escape sequence."); + call_notifier = (level == LOG_BROKEN || level == LOG_UNUSUAL)? true : false; /* FIXME: Let plugin specify node_id? */ - log_(plugin->log, level, NULL, call_notifier, "%.*s", msgtok->end - msgtok->start, - plugin->buffer + msgtok->start); + log_(plugin->log, level, NULL, call_notifier, "%s", msg); return NULL; }