offers: use param_check() for more thorough parameter validation.
What the developer wrote
Authored by Rusty Russell
offers: use param_check() for more thorough parameter validation.
And fix incorrect comment on its use in the header!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
What changed, and why it matters
This commit tightens parameter validation in the 'offers' plugin of Core Lightning. It swaps a less strict validation helper for a stricter one and ensures that when the command is only being checked (not actually executed), the plugin reports completion correctly. The change also fixes an outdated comment describing the stricter helper. There is no direct evidence in the commit of an exploitable security bug, but the change removes a validation gap that could in principle let malformed or incomplete offers slip through.
Review whether any other offer/invoice plugin commands still use param() where param_check() would be more appropriate, and ensure all check-only command paths terminate with command_check_done(). No urgent patch is indicated by this commit alone.
Security signals we found
Validation helper changed from param() to param_check() for stricter parameter checking
Missing explicit command_check_done() path added for check-only commands
Outdated header comment corrected to reflect actual param_check() contract
Evidence from the diff
In plugins/offers_offer.c, json_offer() now uses param_check() instead of param(). param_check() does not automatically fail when command_check_only(cmd) is true, allowing the caller to perform additional validation and then explicitly terminate with command_check_done(). The commit adds that explicit command_check_done() call after the offer-specific sanity checks. The header comment for param_check() in common/json_param.h is corrected to match this contract. The functional effect is that offer creation now undergoes the same thorough JSON parameter validation as other commands, while still supporting dry-run/check-only mode correctly.
Changed components
plugins/offers_offer.ccommon/json_param.hInspect captured patch +32 / −29
diff --git a/common/json_param.h b/common/json_param.h
index 5944550f..e41528a2 100644
--- a/common/json_param.h
+++ b/common/json_param.h
@@ -51,8 +51,8 @@ bool param(struct command *cmd, const char *buffer,
/*
* Version which *doesn't* fail if command_check_only(cmd) is true:
- * allows you can do extra checks after, but MUST still fail with
- * command_param_failed(); if command_check_only(cmd) is true! */
+ * allows you can do extra checks after, but MUST terminate
+ * with command_check_done() if command_check_only(cmd) is true! */
bool param_check(struct command *cmd,
const char *buffer,
const jsmntok_t tokens[], ...) LAST_ARG_NULL;
diff --git a/plugins/offers_offer.c b/plugins/offers_offer.c
index f045b93d..635e26b5 100644
--- a/plugins/offers_offer.c
+++ b/plugins/offers_offer.c
@@ -416,33 +416,33 @@ struct command_result *json_offer(struct command *cmd,
offinfo->offer = offer = tlv_offer_new(offinfo);
- if (!param(cmd, buffer, params,
- p_req("amount", param_amount, offer),
- p_opt("description", param_escaped_string, &desc),
- p_opt("issuer", param_escaped_string, &issuer),
- p_opt("label", param_escaped_string, &offinfo->label),
- p_opt("quantity_max", param_u64, &offer->offer_quantity_max),
- p_opt("absolute_expiry", param_u64, &offer->offer_absolute_expiry),
- p_opt("recurrence", param_recurrence, &offer->offer_recurrence_compulsory),
- p_opt("recurrence_base",
- param_recurrence_base,
- &offer->offer_recurrence_base),
- p_opt("recurrence_paywindow",
- param_recurrence_paywindow,
- &offer->offer_recurrence_paywindow),
- p_opt("recurrence_limit",
- param_number,
- &offer->offer_recurrence_limit),
- p_opt_def("single_use", param_bool,
- &offinfo->single_use, false),
- p_opt_def("proportional_amount",
- param_bool,
- &proportional, false),
- p_opt_def("optional_recurrence",
- param_bool,
- &optional_recurrence, false),
- p_opt("dev_paths", param_paths, &paths),
- NULL))
+ if (!param_check(cmd, buffer, params,
+ p_req("amount", param_amount, offer),
+ p_opt("description", param_escaped_string, &desc),
+ p_opt("issuer", param_escaped_string, &issuer),
+ p_opt("label", param_escaped_string, &offinfo->label),
+ p_opt("quantity_max", param_u64, &offer->offer_quantity_max),
+ p_opt("absolute_expiry", param_u64, &offer->offer_absolute_expiry),
+ p_opt("recurrence", param_recurrence, &offer->offer_recurrence_compulsory),
+ p_opt("recurrence_base",
+ param_recurrence_base,
+ &offer->offer_recurrence_base),
+ p_opt("recurrence_paywindow",
+ param_recurrence_paywindow,
+ &offer->offer_recurrence_paywindow),
+ p_opt("recurrence_limit",
+ param_number,
+ &offer->offer_recurrence_limit),
+ p_opt_def("single_use", param_bool,
+ &offinfo->single_use, false),
+ p_opt_def("proportional_amount",
+ param_bool,
+ &proportional, false),
+ p_opt_def("optional_recurrence",
+ param_bool,
+ &optional_recurrence, false),
+ p_opt("dev_paths", param_paths, &paths),
+ NULL))
return command_param_failed();
/* Doesn't make sense to have max quantity 1. */
@@ -511,6 +511,9 @@ struct command_result *json_offer(struct command *cmd,
offer->offer_recurrence_compulsory = NULL;
}
+ if (command_check_only(cmd))
+ return command_check_done(cmd);
+
/* BOLT #12:
* - if it sets `offer_issuer`:
* - SHOULD set it to identify the issuer of the invoice clearly.
Why this scored 28/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.