| Age | Commit message (Collapse) | Author |
|---|
|
|
|
Previously, no checks were done when __n <= __b, but strncat _appends_ after
existing content, making this a overly broad check check. For example, with an
8-byte buffer containing "12345\0", strncat(buf, "ABCD", 4) would have the
check skipped, but the result "12345ABCD\0" is 10 bytes, resulting in an
overflow.
This commit fixes this oversight, and adds a bunch of tests.
|
|
mbsnrtowcs writes up to __wn wide characters into wchar_t *__d. The destination
capacity is __b / sizeof(wchar_t) wide characters, but the
else branch clamps __n (source byte limit) to __b (destination byte size).
__wn (the actual output count) is passed through unclamped. Example: __b=8
(dest holds 2 wchar_t), __n=100, __wn=25. The else branch applies (25 <=
100/4), clamps source to 8 bytes, but passes __wn=25 β the function can write
25 wchar_t (100 bytes) into an 8-byte buffer.
The first branch is also wrong: it divides __b (bytes) by sizeof(wchar_t) to
get wchar_t capacity, which is correct for the destination β but the condition
__wn > __n / sizeof(wchar_t) uses integer division that can produce incorrect
routing between branches.
The fix mirrors the already-correct mbsrtowcs pattern: clamp __wn (the output
wide-char count) to the destination's wchar_t capacity, and pass __n (source
byte limit) through unchanged.
|
|
|
|
Like it's already done for memcpy and memmove. Add tests as well,
to prove that nothing broke.
|
|
__d is a char * destination buffer, so __b is already the byte capacity.
Dividing by sizeof(wchar_t) makes no sense here, it was likely copy-pasted
from mbsnrtowcs (where the destination is wchar_t *). The first branch also
fails to limit __n (the byte write cap) to __b, so overflows are possible when
a wide character produces multi-byte output. The second branch (else) correctly
limits __n to __b.
This commit replaces the broken two-branch logic with the simple correct
pattern matching wcsrtombs, and adds two tests two prove that nothing broke.
|
|
The manpage says that stpncpy will "copy non-null bytes from the string pointed
to by src into the array pointed to by dst.", it doesn't add a terminal NULL
byte, so we shouldn't check for it.
|
|
Fixes bf242b15e1f7
|
|
gcc does not seem to reliably notice that the if condition makes
overflows impossible in the code. To please the compiler we can use
the __bos flag to return 0 (instead of -1) when the size is unknown.
Fixes https://github.com/jvoisin/fortify-headers/issues/62
Fixes https://github.com/jvoisin/fortify-headers/issues/68
Fixes https://github.com/jvoisin/fortify-headers/issues/80
|
|
Simplifies the code and improves compatibility
|
|
As explained in `man realpath(3)`:
> The POSIX.1-2001 standard version of this function is broken by design,
> since it is impossible to determine a suitable size for the output
> buffer, resolved_path. According to POSIX.1-2001 a buffer of size PATH_MAX
> suffices, but PATH_MAX need not be a defined constant, and may have to be
> obtained using pathconf(3). And asking pathconf(3) does not really help,
> since, on the one hand POSIX warns that the result of pathconf(3) may be huge
> and unsuitable for mallocing memory, and on the other hand pathconf(3) may
> return -1 to signify that PATH_MAX is not bounded. The reβ solved_path
> == NULL feature, not standardized in POSIX.1-2001, but standardized in
> POSIX.1-2008, allows this design problem to be avoided.
So we can either not compile, or be pragmatic, and define PATH_MAX to a sane
value, like 4096, which is the one used on Linux and some/most BSD.
This commit also adds two tests to ensure that things aren't catastrophically
broken by this change.
|
|
|
|
Makes uses of _FORTIFY_INLINE and _FORTIFY_POS0 in sys/select.h,
as clang complains about __gnu_inline__ in some cases. An issue
which is already fixed with the shared macros.
It was reported it:
- https://gitlab.alpinelinux.org/alpine/aports/-/issues/18015
- https://gitlab.alpinelinux.org/alpine/aports/-/issues/18000
Co-Authored-By: Sertonix
|
|
This commit fixes the typo pass_object_size__ for pass_object_size. It also
adds tests in the CI to prevent this from happening again.
Ref https://clang.llvm.org/docs/AttributeReference.html#pass-object-size-pass-dynamic-object-size
Co-Authored-By: Sertonix
|
|
|
|
|
|
LLVM libcxx (20.1.8) also defines a `__format` macro in it's
C++ locale header (`/usr/include/c++/v1/locale`).
Fixes: https://github.com/jvoisin/fortify-headers/issues/76
|
|
|
|
|
|
|
|
|
|
Fixes https://github.com/jvoisin/fortify-headers/issues/31
|
|
It's unfortunately valid to pass a buffer smaller than MB_CUR_MAX to wctomb, so
let's not trap on this. Moreover, it's supposed to be implemented in stdlib.h
and not wchar.h anyway.
|
|
Co-Authored-By: Sertonix <sertonix@posteo.net>
|
|
Co-Authored-By: jvoisin <julien.voisin@dustri.org>
|
|
It seems useless and triggers 'error: expected external declaration'
|
|
fortify/poll.h includes poll.h, which redirects the ppoll sys call to __ppoll_time64,
if the _REDIR_TIME64 macro is 1. Then fortify/poll.h will #undef ppoll and use
the 32 bit version, which is inconsistent.
Taken from: https://github.com/openwrt/openwrt/pull/12575
Co-authored-by: Hauke Mehrtens <hauke@hauke-m.de>
|
|
GCC and Clang provide __builtin_dynamic_object_size
(see documentation: https://gcc.gnu.org/onlinedocs/gcc/Object-Size-Checking.html),
so we should make use of it when its available.
|
|
First, we should never check the size of __s if __l == 0 since the
array is not going to be modified in that case.
Second, negative __l is a well-defined error case (EINVAL) and we
should never trap on a conforming code like this:
r = getgroups(-1, NULL);
if (r == -1)
...
An example of non-desired behaviour for negative __l is the gnulib
configure script which checks for getgroups(-1, ...) to catch some
ancient FreeBSD kernel bug. The conftest binary traps even on good
system (e.g. linux/musl) and the unnecessary getgroups wrapper is
enforced for any project that uses gnulib.
This patch also changes the size_t cast to avoid the explicit zero
extension on systems where size_t differs from unsigned int.
|
|
This allows the compiler to optimize out the slow/trap path at all
for the typical correct code:
char buf[MB_LEN_MAX];
r = wctomb(buf, c);
The change tries to keep the "unknown object size" case handling in
wcrtomb() as is even if it seems redundant and not helping (we copy
__buf to possibly undersized __s in any case) and inconsistent with
wctomb() (where we let the original library method itself overwrite
the possibly undersized __s).
|
|
This allows the compiler to optimize out the slow/trap path at all
for the typical correct code:
char buf[PATH_MAX];
r = realpath(path, buf);
The change keeps the "unknown object size" case handling intact.
|
|
|
|
A few important notes:
* __extension__ is a GNU C "alternate" keyword, not a C++ keyword.[1]
* __extension__ is designed to work on "expressions"; it does work on
#include_next in C mode, but it has no effect in C++ mode; the
warning will still appear, if enabled, even with __extension__
preceding #include_next. This is because #include_next is not
considered an expression in C++, so the compiler attaches
__extension__ to the first expression of the header.
All of this leads us to a build failure while building at least all
Mozilla software. Moz has an alternate -isystem dir searched before
/usr/include that overrides some headers, including <features.h>. The
first statement in each of these headers is a #pragma, and since
__extension__ is looking for an expression, and #pragma is a "null"
expression, we end up with the following error:
dist/system_wrappers/features.h:1:9: error: '#pragma' is not allowed here
Since __extension__ has no effect on #include_next in C++ mode anyway,
and since it can cause breakage, this commit omits __extension__ in C++
mode.
[1]: https://gcc.gnu.org/onlinedocs/gcc-6.4.0/gcc/Alternate-Keywords.html
|
|
The POSIX definition of wcrtomb
(http://pubs.opengroup.org/onlinepubs/9699919799/functions/wcrtomb.html)
states:
"When wc is not a valid wide character, an encoding error shall occur.
In this case, the function shall store the value of the macro [EILSEQ]
in errno and shall return (size_t)-1; the conversion state shall be
undefined."
The fortify-headers implementation of wcrtomb interprets the result -1
as 18446744073709551615 bytes. Since this is the highest 64-bit number
possible, it is pretty safe to say this will always be larger than any
buffer provided to wcrtomb. Therefore, it traps.
Fixes bug https://bugs.alpinelinux.org/issues/7681.
Patch by A. Wilcox <AWilcox@Wilcox-Tech.com>
|
|
|
|
The __extension__ seems to trigger a bug in gcc when there are no
identifier specified afterwards.
Testcase:
echo "#include <stdlib.h>" > try.c && cc -O0 -c try.c
try.c:2:0: error: expected identifier or '(' at end of input
With -O2 it does not happen.
We work around this by only pulling in limits.h when we actually need the
PATH_MAX.
Signed-off-by: Natanael Copa <ncopa@alpinelinux.org>
|
|
|
|
Do not crash unless the overflow would happen.
|
|
Do not crash unless the overflow would actually happen.
|
|
This was breaking valid code, example:
char c;
wcrtomb(&c, L'0', st);
|
|
Signed-off-by: Steven Barth <steven@midlink.org>
|
|
Signed-off-by: Steven Barth <steven@midlink.org>
|
|
Newer compilers default to GNU11, a C11 dialect. Some software however
is unprepared for this or has wrong compatibility checks. What happens
is that some software will for compatibility with C89
#define inline
before inclusion of a standard header, which is undefined behaviour in
C99 and above (C99/C11 7.1.2/4), as inline is a keyword.
If any libc headers that are then included via #include_next provide an
__inline macro definition (current musl does this if C++ or C99 and
above is detected) like the following
#define __inline inline
this results in any __inline token to be preprocessed away.
This breaks use of __builtin_va_arg_pack() in our stdio.h at
compile-time as it can only be used in always inlined functions. The
function attributes __always_inline__ and __gnu_inline__ themselves
require an inline specifier on the function to be applied.
|
|
|
|
The predefined __USER_LABEL_PREFIX__ macro if it is non-empty contains
an identifier, not a string literal, thus it needs to be stringified.
|
|
POSIX specifies them to have return-type void, not int.
|
|
|
|
|
|
|
|
|
