Starting with 75e300aeec ("blobmsg: fix wrong payload len passed from
blobmsg_check_array") blobmsg_check_array_len() gets *blob* length
passed as argument. It cannot be used with __blobmsg_for_each_attr()
which expects *data* length.
Use blobmsg_for_each_attr() which calculates *data* length on its own.
The same bug was already reported in the past and there was fix attempt
in the commit cd75136b13 ("blobmsg: fix wrong payload len passed from
blobmsg_check_array"). That change made blobmsg_check_attr_len() calls
fail however.
This is hopefully the correct & complete fix:
1. blobmsg_check_array_len() gets *blob* length
2. It calls blobmsg_check_attr_len() which requires *blob* length
3. It uses blobmsg_for_each_attr() which gets *data* length
This fixes iterating over random memory treated as attrs. That was
resulting in check failing randomly for totally correct blobs. It's
critical e.g. for procd project with its instance_fill_array() failing
and procd not starting services.
Fixes: 75e300aeec ("blobmsg: fix wrong payload len passed from blobmsg_check_array")
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
Fix incorrect use of blobmsg_len() on passed blobmsg to
blobmsg_check_array_len() introduced in commit 379cd33d19
("fix wrong payload len passed from blobmsg_check_array") by using correct
blob_len().
By using blobmsg_len() a value too small was passed to blobmsg_check_array()
which could lead to this function returning an error when there is none.
Fixes: 379cd33d19 ("fix wrong payload len passed from blobmsg_check_array")
Signed-off-by: Chris Nisbet <nischris@gmail.com>
[add fixes tag, rewrap commit message]
Signed-off-by: Jo-Philipp Wich <jo@mein.io>
Fix out of bounds read in blobmsg_parse and blobmsg_check_name. The
out of bounds read happens because blob_attr and blobmsg_hdr have
flexible array members, whose size is 0 in the corresponding sizeofs.
For example the __blob_for_each_attr macro checks whether rem >=
sizeof(struct blob_attr). However, what LibFuzzer discovered was,
if the input data was only 4 bytes, the data would be casted to blob_attr,
and later on blob_data(attr) would be called even though attr->data was empty.
The same issue could appear with data larger than 4 bytes, where data
wasn't empty, but contained only the start of the blobmsg_hdr struct,
and blobmsg_hdr name was empty. The bugs were discovered by fuzzing
blobmsg_parse and blobmsg_array_parse with LibFuzzer.
CC: Luka Perkov <luka.perkov@sartura.hr>
Reviewed-by: Jo-Philipp Wich <jo@mein.io>
Signed-off-by: Juraj Vijtiuk <juraj.vijtiuk@sartura.hr>
[refactored some checks, added fuzz inputs, adjusted unit test results]
Signed-off-by: Petr Štetiar <ynezz@true.cz>
Better safe than sorry and while at it add handling of possible
*printf() failures.
Reviewed-by: Jo-Philipp Wich <jo@mein.io>
Signed-off-by: Petr Štetiar <ynezz@true.cz>
Fix incorrect use of blob_raw_len() on passed blobmsg to
blobmsg_check_array_len() introduced in commit b0e21553ae ("blobmsg:
add _len variants for all attribute checking methods") by using correct
blobmsg_len().
This wrong (higher) length was then for example causing issues in
procd's instance_config_parse_command() where blobmsg_check_attr_list()
was failing sanity checking of service command, thus resulting in the
startup failures of some services like collectd, nlbwmon and samba4.
Ref: http://lists.infradead.org/pipermail/openwrt-devel/2019-December/020840.html
Fixes: b0e21553ae ("blobmsg: add _len variants for all attribute checking methods")
Reported-by: Hannu Nyman <hannu.nyman@welho.com>
Tested-by: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>
Signed-off-by: Petr Štetiar <ynezz@true.cz>
Fixes following warning reported by GCC 10.0.0 20191203:
blobmsg.c:234:2: error: 'strcpy' offset 6 from the object at 'attr' is out of the bounds of referenced subobject 'name' with type 'uint8_t[0]' {aka 'unsigned char[0]'} at offset 6 [-Werror=array-bounds]
234 | strcpy((char *) hdr->name, (const char *)name);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from blobmsg.c:16:
blobmsg.h:42:10: note: subobject 'name' declared here
42 | uint8_t name[];
| ^~~~
Reported-by: Khem Raj <raj.khem@gmail.com>
Signed-off-by: Petr Štetiar <ynezz@true.cz>
Introduce _len variants of blobmsg attribute checking functions which
aims to provide safer implementation as those functions should limit all
memory accesses performed on the blob to the range [attr, attr + len]
(upper bound non inclusive) and thus should be suited for checking of
untrusted blob attributes.
While at it add some comments in order to make it clear.
Signed-off-by: Tobias Schramm <tobleminer@gmail.com>
[_safe -> _len, blobmsg_check_array_len fix, commit subject/desc facelift]
Signed-off-by: Petr Štetiar <ynezz@true.cz>
blobmsg_check_attr_len adds a length limit specifying the max offset
from attr that can be read safely.
Signed-off-by: Tobias Schramm <tobleminer@gmail.com>
[rebased and reworked, line wrapped commit message, _safe -> _len]
Signed-off-by: Petr Štetiar <ynezz@true.cz>
Fixes following error found by the fuzzer:
==29774==ERROR: AddressSanitizer: heap-buffer-overflow
READ of size 1 at 0x6020004f1c56 thread T0
#0 strcmp sanitizer_common_interceptors.inc:442:3
#1 blobmsg_parse blobmsg.c:168:8
Signed-off-by: Petr Štetiar <ynezz@true.cz>
Fixes following compiler warnings:
blobmsg.c:242:39: error: format string is not a string literal [-Werror,-Wformat-nonliteral]
blobmsg.c:248:23: error: format string is not a string literal [-Werror,-Wformat-nonliteral]
ulog.c💯18: error: format string is not a string literal [-Werror,-Wformat-nonliteral]
ulog.c:112:16: error: format string is not a string literal [-Werror,-Wformat-nonliteral]
ulog.c:117:20: error: format string is not a string literal [-Werror,-Wformat-nonliteral]
Signed-off-by: Petr Štetiar <ynezz@true.cz>
gcc-9 on x86/64 has reported following issues:
base64.c:173:17: error: comparison of integer expressions of different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare]
base64.c:230:18: error: comparison of integer expressions of different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare]
base64.c:238:18: error: comparison of integer expressions of different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare]
base64.c:242:22: error: comparison of integer expressions of different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare]
base64.c:252:18: error: comparison of integer expressions of different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare]
base64.c:256:22: error: comparison of integer expressions of different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare]
base64.c:266:18: error: comparison of integer expressions of different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare]
base64.c:315:27: error: comparison of integer expressions of different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare]
base64.c:329:15: error: comparison of integer expressions of different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare]
blob.c:207:11: error: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘int’ [-Werror=sign-compare]
blob.c:210:11: error: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘int’ [-Werror=sign-compare]
blob.c:243:31: error: comparison of integer expressions of different signedness: ‘int’ and ‘unsigned int’ [-Werror=sign-compare]
blob.c:246:31: error: comparison of integer expressions of different signedness: ‘int’ and ‘unsigned int’ [-Werror=sign-compare]
blob.h:245:37: error: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘int’ [-Werror=sign-compare]
blob.h:253:37: error: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘int’ [-Werror=sign-compare]
blobmsg.h:269:37: error: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘int’ [-Werror=sign-compare]
blobmsg_json.c:155:10: error: comparison of integer expressions of different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare]
examples/../blob.h:245:37: error: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘int’ [-Werror=sign-compare]
examples/../blobmsg.h:269:37: error: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘int’ [-Werror=sign-compare]
json_script.c:590:7: error: this statement may fall through [-Werror=implicit-fallthrough=]
Signed-off-by: Petr Štetiar <ynezz@true.cz>
This adds support for double floating point type to make it more JSON
compatible. For type checking it also adds a stub BLOB_ATTR_DOUBLE type.
If necessary, the accessor functions for blob can be added later
Signed-off-by: André Gaul <andre@gaul.io>
Signed-off-by: Felix Fietkau <nbd@nbd.name>
Consistently handle allocation failures. Some functions are changed to
return bool or int instead of void to allow returning an error.
Also fix a buffer size miscalculation in lua/uloop and use _exit() instead
of exit() on errors after forking.
Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
This primarily helps with simplifying the ubus APIs.
blobmsg header presence is indicated by the BLOB_ATTR_EXTENDED bit in
the id_len field.
This changes the format ABI, but not the API.
Signed-off-by: Felix Fietkau <nbd@openwrt.org>
