==31775==ERROR: AddressSanitizer: SEGV on unknown address 0x604000a7c715
==31775==The signal is caused by a READ memory access.
#0 blobmsg_check_attr blobmsg.c:48:6
#1 blobmsg_parse_array blobmsg.c:118:8
#2 fuzz_blobmsg_parse test-blobmsg-parse-fuzzer.c:35:2
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>
blob_parse can be only used on trusted input as it has no possibility to
check the length of the provided input buffer, which might lead to
undefined behaviour and/or crashes when supplied with malformed,
corrupted or otherwise specially crafted input.
So this introduces blob_parse_untrusted variant which expects additional
input buffer length argument and thus should be able to process also
inputs from untrusted sources.
Signed-off-by: Petr Štetiar <ynezz@true.cz>
==5872==ERROR: AddressSanitizer: SEGV on unknown address 0x6020004100b4
==5872==The signal is caused by a READ memory access.
#0 blob_data blob.h
#1 blob_parse blob.c:228:2
Signed-off-by: Petr Štetiar <ynezz@true.cz>
LibFuzzer is in-process, coverage-guided, evolutionary fuzzing engine.
LibFuzzer is linked with the library under test, and feeds fuzzed inputs
to the library via a specific fuzzing entrypoint (aka "target
function"); the fuzzer then tracks which areas of the code are reached,
and generates mutations on the corpus of input data in order to maximize
the code coverage.
Lets use libFuzzer to fuzz blob and blobmsg parsing for the start.
Ref: https://llvm.org/docs/LibFuzzer.html
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>
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 version 4.8.4 (Ubuntu 14.04) and -Wextra produces following:
json_script.c:124:3: error: missing initializer for field 'name' of 'struct blobmsg_policy' [-Werror=missing-field-initializers]
Reported-by: Jonas Gorski <jonas.gorski@gmail.com>
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>
Fixes following error:
Invalid read of size 1
at 0x4C32D04: strlen
by 0x5043367: json_tokener_parse_ex
by 0x5045316: json_tokener_parse_verbose
by 0x504537D: json_tokener_parse
by 0x401AB1: jshn_parse (jshn.c:179)
by 0x40190D: jshn_parse_file (jshn.c:370)
by 0x40190D: main (jshn.c:434)
Address 0x5848c4c is 0 bytes after a block of size 1,036 alloc'd
at 0x4C2FB0F: malloc
by 0x4018E2: jshn_parse_file (jshn.c:357)
by 0x4018E2: main (jshn.c:434)
Signed-off-by: Petr Štetiar <ynezz@true.cz>
Fixes following leaks of memory:
352 (72 direct, 280 indirect) bytes in 1 blocks are definitely lost in loss record 3 of 3
at 0x4C31B25: calloc
by 0x5042E1F: json_object_new_array
by 0x5044B02: json_tokener_parse_ex
by 0x5045316: json_tokener_parse_verbose
by 0x504537D: json_tokener_parse
by 0x401AA9: jshn_parse (jshn.c:179)
by 0x401977: main (jshn.c:378)
752 (72 direct, 680 indirect) bytes in 1 blocks are definitely lost in loss record 6 of 6
at 0x4C31B25: calloc
by 0x50424CF: json_object_new_object
by 0x5044B38: json_tokener_parse_ex
by 0x5045316: json_tokener_parse_verbose
by 0x504537D: json_tokener_parse
by 0x401AA9: jshn_parse (jshn.c:179)
by 0x401977: main (jshn.c:380)
Signed-off-by: Petr Štetiar <ynezz@true.cz>
Fixes following leak of memory:
6,016 bytes in 1 blocks are possibly lost in loss record 1 of 1
at 0x4C31B25: calloc
by 0x1098F8: main (jshn.c:353)
Signed-off-by: Petr Štetiar <ynezz@true.cz>
Turn longer switch cases into separate functions in order to make it
easier to follow. Don't return from the cases as it makes future
cleaning up harder.
Signed-off-by: Petr Štetiar <ynezz@true.cz>
clang-10 analyzer reports following:
avl.c:671:25: warning: Access to field 'parent' results in a dereference of a null pointer (loaded from field 'right')
node->right->parent = parent;
~~~~~ ^
Which seems to be impossible to trigger via exported AVL public API, but
it could be probably trigerred by fiddling with the AVL tree node struct
members manually as they are exposed.
Signed-off-by: Petr Štetiar <ynezz@true.cz>
clang-10 analyzer reports following:
blobmsg_json.c:285:2: warning: The expression is an uninitialized value. The computed value will also be garbage
s->indent_level++;
^~~~~~~~~~~~~~~~~
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>
In order to allow seamless assert() usage in release builds without the
need for fiddling with CMake C flags as CMake adds -DNDEBUG switch in
release builds which disable assert().
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>
Uses currently proof-of-concept openwrt-ci[1] in order to:
* improve the quality of the codebase in various areas
* decrease code review time and help merging contributions faster
* get automagic feedback loop on various platforms and tools
- out of tree build with OpenWrt SDK on following targets:
* ath79-generic
* imx6-generic
* malta-be
* mvebu-cortexa53
- out of tree native build on x86/64 with GCC (versions 7, 8, 9) and Clang 10
- out of tree native x86/64 static code analysis with cppcheck and
scan-build from Clang 10
1. https://gitlab.com/ynezz/openwrt-ci/
Signed-off-by: Petr Štetiar <ynezz@true.cz>
Let's enforce additional automatic checks enforced by the compiler in
order to catch possible errors during compilation.
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 would allow board_config_flush to run one command instead
of two and would be faster and safer than redirecting output
and moving a file between filesystems.
Originally discussed here:
http://lists.openwrt.org/pipermail/openwrt-devel/2017-December/010127.html
Signed-off-by: Roman Yeryomin <roman@advem.lv>
This tells the compiler that these functions are takeing a format
string, the compiler will now do additional checks and is able to emitt
a compile warning in case the format string is not valid.
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
This offers an increased level of security, as the arguments will be
checked for validity against the format string at compile time. The
format attribute is supported by both GCC and Clang, so there shouldn't
be any portability issues.
Signed-off-by: Kristupas Savickas <savickas.kristupas@gmail.com>
Some gcc versions have issues with the __builtin_choose_expr construct
which attempts to automatically use the const versions of those
functions.
Make it possible to explicitly use const_* versions to avoid that.
Signed-off-by: Felix Fietkau <nbd@nbd.name>
g++ does not support __builtin_choose_expr, so we can't support
byte swapping as constant expression there.
Reported-by: Cleynhens Stijn <Stijn.Cleynhens@technicolor.com>
Signed-off-by: Felix Fietkau <nbd@nbd.name>
Simplifies portability and ensures that cpu_to_* can be used in const
declarations. If the architecture has special instructions, the compiler
should be able to detect the pattern and use them.
Signed-off-by: Felix Fietkau <nbd@nbd.name>
Function usefull to iterate through the different elements of an
array or object; the provided callback function is called for each
element which is passed the value, key and user provided arguments.
For field types different from array or object the callback is called
with the retrieved value.
Signed-off-by: Hans Dedecker <dedeckeh@gmail.com>
Acked-by: Jo-Philipp Wich <jo@mein.io>
The existing read functionality feeds the complete JSON to jshn as a
cmdline argument, leading to `-ash: jshn: Argument list too long`
errors for JSONs bigger than ca. 100KB.
This commit adds the ability to read the JSON directly from a file if
wanted, removing this shell-imposed size limit.
Tested on x86-64 and ar71xx. An mmap()-based solution was also evaluated,
but found to make no performance difference on either platform.
Signed-off-by: Christian Beier <dontmind@freeshell.org>
Instead of abort parsing, properly deal with "null" values by implementing
support for reading and formatting such values.
Signed-off-by: Jo-Philipp Wich <jo@mein.io>
This allows for reading in and writing out bigger JSON Numbers.
Following test script (that fails to print correct values _without_ this
commit) verifies the functionality (tested on x86-64 as well as on ar71xx):
---snip---
# assumes you built jshn and sourced jshn.sh
echo testing reading-in JSON
SHELL_BIGNUM=12147483647
json_init
json_load "{ \"bignum\": $SHELL_BIGNUM }"
json_get_var BIGNUM bignum
echo jshn bignum: $BIGNUM
echo shll bignum: $SHELL_BIGNUM
echo testing writing-out JSON
json_init
json_add_int bigint $SHELL_BIGNUM
json_dump
--snap---
Signed-off-by: Christian Beier <dontmind@freeshell.org>
The bitfield functions and macros were committed without explaining
their purpose in the commit message.
As they are only used in uci, and conflict with similar functions added
in hostapd, breaking our hostapd ubus patch, nuke them from libubox and
add them in uci instead.
If we need them anywhere else in the future we can add it to libubox
again, but preferably prefixed with ubox_.
Signed-off-by: Stijn Tintel <stijn@linux-ipv6.be>
Some programs want to manage their own child life cycle without using
SIGCHLD signal handler. In these cases, uloop is reaping children for
them because they don't have SIGCHLD handler set. This patch makes it
possible to disable reaping children through 'uloop_handle_sigchld'
variable.
Signed-off-by: Petar Paradzik <petar.paradzik@sartura.hr>
When uloop_process_add() is called outside of uloop_run(), i.e. not
from a callback (which is the case of at least utrace and ujail),
child events can be missed. The reason is that when SIGCHILD handler
is installed in uloop_run(), after the uloop_process_add() is called,
then an initial signal could be missed.
Commit 4e3a47a ("uloop: use a waker for notifying sigchld and loop
cancel events", 2016-06-09) solved a similar problem and introduced
uloop_init() but forgot to move a call to uloop_setup_signals() there.
This is what this commit does.
Now, uloop_process_add() can be called any time after uloop_init()
without missing any event.
Signed-off-by: Michal Sojka <sojkam1@fel.cvut.cz>
Acked-by: Yousong Zhou <yszhou4tech@gmail.com>
Variable confusion was breaking timers
Fixes: 368fd26458 ("uloop: allow specifying a timeout for uloop_run()")
Signed-off-by: Felix Fietkau <nbd@nbd.name>
Calling t->complete in runqueue_task_complete can free the memory
associated with t. Change the runqueue_start_next accordingly.
Fixes https://github.com/openwrt/openwrt/issues/493
Signed-off-by: Felix Fietkau <nbd@nbd.name>
This can be useful for cleanup with pending timers, or for hooking into
existing code that does not use uloop
Signed-off-by: Felix Fietkau <nbd@nbd.name>
