On Mon, Mar 31, 2025 at 02:37:15PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
> > Hi Florian,
> >
> > On Thu, Mar 27, 2025 at 04:17:11PM +0100, Florian Westphal wrote:
> > > The bogon will trigger the assertion in mpz_import_data:
> > > src/expression.c:418: constant_expr_alloc: Assertion `(((len) + (8) - 1) / (8)) > 0' failed.
> >
> > I took a quick look searching for {s:s} in src/parser_json.c
> >
> > The common idiom is json_parse_err() then a helper parser function to
> > validate the string.
> >
> > It seems it is missing in this case. Maybe tigthen json parser instead?
> >
> > Caller invoking constant_expr_alloc() with data != NULL but no len
> > looks broken to me.
>
> return constant_expr_alloc(int_loc, &string_type, BYTEORDER_HOST_ENDIAN,
> strlen(chain) * BITS_PER_BYTE, chain);
>
> chain name is '""'.
>
> There are other spots where we possibly call into constant_expr_alloc()
> with a 0 argument.
Reviewed-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
> I think it would be a lot more work and bloat to add all the checks on
> the json side while its a one-liner in constant_expr_alloc().
I don't think this is needed, the idiom in src/parser_json.c is to:
1) fetch string
2) validate it
For example:
if (!json_unpack(root, "{s:s}", "ttl", &ttl)) {
if (!strcmp(ttl, "loose")) {
ttlval = 1;
} else if (!strcmp(ttl, "skip")) {
ttlval = 2;
} else {
json_error(ctx, "Invalid osf ttl option '%s'.", ttl);
return NULL;
}
}
> I could also add json_constant_expr_alloc() but it seems kinda silly to me.
I agree, I don't think json_constant_expr_alloc is needed.
