lightningd: unescape log message strings received from plugins

ccan/ccan/json_escape/json_escape.c

@@ -1,6 +1,7 @@
 /* MIT (BSD) license - see LICENSE file for details */
 #include <ccan/json_escape/json_escape.h>
 #include <stdio.h>
+#include <ccan/tal/str/str.h>
 
 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);
+}

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 */

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;
 	}

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);

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);

lightningd/plugin.c

@@ -3,6 +3,7 @@
 #include <ccan/ccan/tal/grab_file/grab_file.h>
 #include <ccan/crc32c/crc32c.h>
 #include <ccan/io/io.h>
+#include <ccan/json_escape/json_escape.h>
 #include <ccan/mem/mem.h>
 #include <ccan/opt/opt.h>
 #include <ccan/pipecmd/pipecmd.h>
@@ -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;
 }
 

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;