[OpenWrt-Devel] [PATCH procd 3/4] system: sysupgrade: rework firmware validation

[Hauke Mehrtens]

On 1/5/20 11:40 AM, Petr Štetiar wrote:
> Hauke Mehrtens <hauke at hauke-m.de> [2020-01-04 20:41:44]:
> 
> Hi,
> 
> thanks for the review!
> 
>> Please annotate the function with:
>> __attribute__ ((format (printf, 2, 3)));
> 
> Done.
> 
>>> +	va_start(va, fmt);
>>> +	r = vsnprintf(buf+r, sizeof(buf)-r, fmt, va);
>>
>> Please check here for truncation:
>>
>> rv = vsnprintf(buf+r, sizeof(buf)-r, fmt, va);
>> if (rv < 0 || rv >=  sizeof(buf)-r ) {
> 
> I think, that it's better to get truncated message to 256B (if we hit this
> corner cases, we can increase the buffer), then "vsnprintf error".

Fine with me, but please NULL terminate the string, or use
"sizeof(buf) - r - 1" as the size as you already NULL the string before.

> 
>>> +		blobmsg_add_object(&b, jsobj);
>>> +		json_object_put(jsobj);
>>> +		return VJSON_SUCCESS;
>>> +	}
>>> +
>>> +	return vjson_error(err, "failed to parse JSON: %s (%d)",
>>> +			   json_tokener_error_desc(json_tokener_get_error(tok)),
>>> +			   json_tokener_get_error(tok));
>>
>> Why don't you free it here too json_object_put()?
> 
> It should be NULL, json_tokener_parse_ex returns object only in case it
> returns json_tokener_success.

Ok

Hauke

_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel