From 981fd2326a64945e7096c46fce8a2732f3894bc6 Mon Sep 17 00:00:00 2001 From: Simon Vrouwe Date: Wed, 15 Jun 2022 08:47:57 +0300 Subject: [PATCH] lightningd: convert plugin->cmd to absolute path, fail plugin early when non-existent Otherwise different relative paths (e.g. /dir/plugin and /dir/../dir/plugin) to same plugin executable would not be recognized as the same plugin --- lightningd/options.c | 10 ++++++++-- lightningd/plugin.c | 14 ++++++++++++-- lightningd/plugin_control.c | 4 ++-- tests/test_plugin.py | 3 ++- 4 files changed, 24 insertions(+), 7 deletions(-) diff --git a/lightningd/options.c b/lightningd/options.c index 454131f3e..c1ec6ac3e 100644 --- a/lightningd/options.c +++ b/lightningd/options.c @@ -458,11 +458,14 @@ static char *opt_add_proxy_addr(const char *arg, struct lightningd *ld) static char *opt_add_plugin(const char *arg, struct lightningd *ld) { + struct plugin *p; if (plugin_blacklisted(ld->plugins, arg)) { log_info(ld->log, "%s: disabled via disable-plugin", arg); return NULL; } - plugin_register(ld->plugins, arg, NULL, false, NULL, NULL); + p = plugin_register(ld->plugins, arg, NULL, false, NULL, NULL); + if (!p) + return tal_fmt(NULL, "Failed to register %s: %s", arg, strerror(errno)); return NULL; } @@ -485,11 +488,14 @@ static char *opt_clear_plugins(struct lightningd *ld) static char *opt_important_plugin(const char *arg, struct lightningd *ld) { + struct plugin *p; if (plugin_blacklisted(ld->plugins, arg)) { log_info(ld->log, "%s: disabled via disable-plugin", arg); return NULL; } - plugin_register(ld->plugins, arg, NULL, true, NULL, NULL); + p = plugin_register(ld->plugins, arg, NULL, true, NULL, NULL); + if (!p) + return tal_fmt(NULL, "Failed to register %s: %s", arg, strerror(errno)); return NULL; } diff --git a/lightningd/plugin.c b/lightningd/plugin.c index 3ad5e330e..5cb6d08b6 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -253,11 +253,16 @@ struct plugin *plugin_register(struct plugins *plugins, const char* path TAKES, const jsmntok_t *params STEALS) { struct plugin *p, *p_temp; + char* abspath; u32 chksum; + abspath = path_canon(tmpctx, path); + if (!abspath) { + return NULL; + } /* Don't register an already registered plugin */ list_for_each(&plugins->plugins, p_temp, list) { - if (streq(path, p_temp->cmd)) { + if (streq(abspath, p_temp->cmd)) { /* If added as "important", upgrade to "important". */ if (important) p_temp->important = true; @@ -276,7 +281,7 @@ struct plugin *plugin_register(struct plugins *plugins, const char* path TAKES, p = tal(plugins, struct plugin); p->plugins = plugins; - p->cmd = tal_strdup(p, path); + p->cmd = tal_steal(p, abspath); p->checksum = file_checksum(plugins->ld, p->cmd); p->shortname = path_basename(p, p->cmd); p->start_cmd = start_cmd; @@ -912,6 +917,11 @@ static const char *plugin_opt_add(struct plugin *plugin, const char *buffer, popt->name = tal_fmt(popt, "--%.*s", nametok->end - nametok->start, buffer + nametok->start); + + /* an "|" alias could circumvent the unique-option-name check */ + if (strchr(popt->name, '|')) + return tal_fmt(plugin, "Option \"name\" may not contain '|'"); + popt->description = NULL; if (deptok) { if (!json_to_bool(buffer, deptok, &popt->deprecated)) diff --git a/lightningd/plugin_control.c b/lightningd/plugin_control.c index bee3ef56c..6fbaf2a4e 100644 --- a/lightningd/plugin_control.c +++ b/lightningd/plugin_control.c @@ -71,8 +71,8 @@ plugin_dynamic_start(struct plugin_command *pcmd, const char *plugin_path, if (!p) return command_fail(pcmd->cmd, JSONRPC2_INVALID_PARAMS, - "%s: already registered", - plugin_path); + "%s: %s", plugin_path, + errno ? strerror(errno) : "already registered"); /* This will come back via plugin_cmd_killed or plugin_cmd_succeeded */ err = plugin_send_getmanifest(p); diff --git a/tests/test_plugin.py b/tests/test_plugin.py index 78cd3fdb8..649544b1f 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -2074,10 +2074,11 @@ def test_important_plugin(node_factory): n = node_factory.get_node(options={"important-plugin": os.path.join(pluginsdir, "nonexistent")}, may_fail=True, expect_fail=True, allow_broken_log=True, start=False) + n.daemon.start(wait_for_initialized=False) # Will exit with failure code. assert n.daemon.wait() == 1 - assert n.daemon.is_in_stderr(r"error starting plugin '.*nonexistent'") + assert n.daemon.is_in_stderr(r"Failed to register .*nonexistent: No such file or directory") # Check we exit if the important plugin dies. n.daemon.opts['important-plugin'] = os.path.join(pluginsdir, "fail_by_itself.py")