The blob buffer has no limitation in place
to prevent buflen to exceed maximum size.
This commit adds a test to demonstrate how
a blob increases past the maximum allowd
size of 16MB. It continuously adds chunks
of 64KB and with the 255th one blob_add()
returns a valid attribute pointer but the
blob's buflen does not increase.
The test is used to demonstrate the
failure, which is fixed with a follow-up
commit.
Signed-off-by: Zefir Kurtisi <zefir.kurtisi@gmail.com>
[adjusted test case for cram usage]
Signed-off-by: Petr Štetiar <ynezz@true.cz>
Following regression was introduced in commit 5e75160f48 ("blobmsg:
fix attrs iteration in the blobmsg_check_array_len()"):
Thread 1 "test-fuzz" received signal SIGSEGV, Segmentation fault.
in blob_len (attr=0x6020000100d4) at libubox/blob.h:102
102 return (be32_to_cpu(attr->id_len) & BLOB_ATTR_LEN_MASK) - sizeof(struct blob_attr);
blob_len (attr=0x6020000100d4) at /libubox/blob.h:102
blob_raw_len (attr=0x6020000100d4) at /libubox/blob.h:111
blob_pad_len (attr=0x6020000100d4) at /libubox/blob.h:120
blobmsg_check_array_len (attr=0x6020000000d0, type=0, blob_len=10) at /libubox/blobmsg.c:145
fuzz_blobmsg_parse (data=0x6020000000d0 "\001\004", size=10) at /libubox/tests/fuzz/test-fuzz.c:57
Signed-off-by: Petr Štetiar <ynezz@true.cz>
Some tools like ucert use concatenations of multiple blobs. Account for
this case by allowing the underlying buffer length to be greater than
the blob length.
Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
Fixes a use-after-free bug in runqueue_task_kill():
Invalid read of size 8
at runqueue_task_kill (runqueue.c:200)
by uloop_process_timeouts (uloop.c:505)
by uloop_run_timeout (uloop.c:542)
by uloop_run (uloop.h:111)
by main (tests/test-runqueue.c:126)
Address 0x5a4b058 is 24 bytes inside a block of size 208 free'd
at free
by runqueue_task_complete (runqueue.c:234)
by runqueue_task_kill (runqueue.c:199)
by uloop_process_timeouts (uloop.c:505)
by uloop_run_timeout (uloop.c:542)
by uloop_run (uloop.h:111)
by main (tests/test-runqueue.c:126)
Block was alloc'd at
at calloc
by add_sleeper (tests/test-runqueue.c:101)
by main (tests/test-runqueue.c:123)
Since commit 11e8afea (runqueue should call the complete handler from
more places) the call to the complete() callback has been moved to
runqueue_task_complete(). However in runqueue_task_kill()
runqueue_task_complete() is called before the kill() callback. This
will result in a use after free if the complete() callback frees the
task struct.
Furthermore runqueue_start_next() is already called at the end of
runqueue_task_complete(), so there is no need to call it again in
runqueue_task_kill().
The issue was that the _complete() callback frees the memory used by the
task struct, which is then read after the _complete() callback returns.
Ref: FS#3016
Signed-off-by: Alban Bedel <albeu@free.fr>
[initial test case, kill cb comment fix]
Signed-off-by: Chris Nisbet <nischris@gmail.com>
[testcase improvements and commit subject/description tweaks]
Signed-off-by: Petr Štetiar <ynezz@true.cz>
* add a test for blobmsg_check_array() to test an array with a string in it
This test was added in conjunction with a change to blobmsg_check_array() to
get it to pass the length obtained from blob_len() rather than blobmsg_len().
Signed-off-by: Chris Nisbet <nischris@gmail.com>
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 prefer use of PRId16 and PRId32
formatting constants as well.
Reviewed-by: Jo-Philipp Wich <jo@mein.io>
Signed-off-by: Petr Štetiar <ynezz@true.cz>
libjson-c json_type_int values are stored as int64_t. Use
json_object_get_int64() instead of json_object_get_int()
to avoid clamping to INT32_MAX.
Reviewed-by: Jo-Philipp Wich <jo@mein.io>
Signed-off-by: Dainis Jonitis <dainis.jonitis@ubnt.com>
[fixed author to match SoB, added unit test results]
Signed-off-by: Petr Štetiar <ynezz@true.cz>
int16 blobmsg type is currently being serialized as uint16_t due to
missing cast during JSON output.
Following blobmsg content:
bar-min: -32768 (i16)
bar-max: 32767 (i16)
Produces following JSON:
{ "bar-min":32768,"bar-max":32767 }
Whereas one would expect:
{ "bar-min":-32768,"bar-max":32767 }
Reviewed-by: Jo-Philipp Wich <jo@mein.io>
Signed-off-by: Petr Štetiar <ynezz@true.cz>
* add missing test with sanitizers
* add test case for blobmsg_add_json_from_string
* add test cases for all numeric types
* print types for each variable
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>
Found by fuzzer:
ERROR: AddressSanitizer: SEGV on unknown address 0x602100000455
The signal is caused by a READ memory access.
#0 in blob_check_type blob.c:214:43
#1 in blob_parse_attr blob.c:234:9
#2 in blob_parse_untrusted blob.c:272:12
#3 in fuzz_blob_parse tests/fuzzer/test-blob-parse-fuzzer.c:34:2
#4 in LLVMFuzzerTestOneInput tests/fuzzer/test-blob-parse-fuzzer.c:39:2
Caused by following line:
if (type == BLOB_ATTR_STRING && data[len - 1] != 0)
where len was pointing outside of the data buffer.
Signed-off-by: Petr Štetiar <ynezz@true.cz>
Currently we run all tests via Valgrind. This patch adds 2nd batch of
tests which are compiled with Clang AddressSanitizer[1],
LeakSanitizer[2] and UndefinedBehaviorSanitizer[3] in order to catch
more issues during QA on CI.
AddressSanitizer is a fast memory error detector. The tool can detect
the following types of bugs:
* Out-of-bounds accesses to heap, stack and globals
* Use-after-free, use-after-return, use-after-scope
* Double-free, invalid free
LeakSanitizer is a run-time memory leak detector. It can be combined
with AddressSanitizer to get both memory error and leak detection, or
used in a stand-alone mode.
UndefinedBehaviorSanitizer (UBSan) is a fast undefined behavior
detector. UBSan modifies the program at compile-time to catch various
kinds of undefined behavior during program execution, for example:
* Using misaligned or null pointer
* Signed integer overflow
* Conversion to, from, or between floating-point types which would
overflow the destination
1. http://clang.llvm.org/docs/AddressSanitizer.html
2. http://http://clang.llvm.org/docs/LeakSanitizer.html
3. http://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html
Signed-off-by: Petr Štetiar <ynezz@true.cz>
Add missing usage hints for -p and -o arguments.
Fixes: e16fa068a5 ("jshn: add support for namespaces")
Fixes: eb30a03048 ("libubox, jshn: add option to write output to a file")
Signed-off-by: Petr Štetiar <ynezz@true.cz>
clang-10 analyzer reports following:
base64.c:325:20: warning: Array access (from variable 'target') results in a null pointer dereference
target[tarindex] = 0;
~~~~~~ ^
and prepared test case confirms it:
Invalid write of size 1
at 0x4E4463F: b64_decode (base64.c:325)
by 0x40088C: test_invalid_inputs (tests/test-base64.c:26)
by 0x40088C: main (tests/test-base64.c:32)
Address 0x1 is not stack'd, malloc'd or (recently) free'd
Process terminating with default action of signal 11 (SIGSEGV)
Access not within mapped region at address 0x1
at 0x4E4463F: b64_decode (base64.c:325)
by 0x40088C: test_invalid_inputs (tests/test-base64.c:26)
by 0x40088C: main (tests/test-base64.c:32)
Signed-off-by: Petr Štetiar <ynezz@true.cz>
For improved QA etc. For the start with initial test cases for avl,
base64, jshn and list components. Moved runqueue and blobmsg from
examples to tests. Converted just a few first test cases from
json-script example into the new cram based unit test, more to come.
Signed-off-by: Petr Štetiar <ynezz@true.cz>
