Commit 419ca82d57 (Fix m17n backspace handling causes out-of-bounds
write in checkType) introduced an incomplete fix.
In function checkType we store the length of the previous multi-char
character in a buffer plens_buffer with pointer plens pointing to the
current position inside the buffer. When encountering a backspace plens
is set to the previous position without a bounds check. This will lead
to plens being out of bounds if we get more backspaces than we have
processed multi-char characters before.
If we are at the beginning of the buffer do not decrement and set plen
(the current length) to 0.
This also fixes GH Issue #270 [BUG] Out of bound read in Strnew_size ,
Str.c:61
If the above explanation does sound weird it's because I didn't fully
grok that function. :-)
When building with gcc and -Wnull-dereference, -O3 and -flto we get a
lot of warnings about potential null dereferences. Exiting instead of
returning NULL solves all of it.
If we cannot alloc memory, there is not much to do - exiting is the
easiest option.
Introduce a separate TU for utility functions util.c. Add a function
exec_cmd to simplify execution of system commands with error handling.
While at it, suppress a warning about unused result when executing a
shell command. As we only display the command's output we do not care
about the exit code.
This fixes Debian bug #398989[0]
[0]: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=398989
Like less, w3m can use an input preprocessor when displaying files. The
preprocessor command is taken from the environment variable LESSOPEN.
The command line in LESSOPEN should include one occurrence of the string
"%s", which will be replaced by the filename when the input preprocessor
command is invoked. Giving more than one "%s" - or a any other
conversion specifier - will lead to undefined behaviour.
Add a check to make sure the command given has only one "%s".
This fixes https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=991608
w3m reads the history file on startup and writes it on exit. That means
if you open multiple instances of w3m, the history file will contain the
history of the last instance closed. All other history changes are lost.
Check if the modification time of the history file has changed before
writing. If it has changed read the history file from the disk into a
new history. Push the entries that are in the current history but not in
the history file into the new history and write the new history to disk.
GCC 12.2.0 on Arch reports:
.table.c: In function ???set_table_matrix0???:
.table.c:3536:5: error: ???__builtin_memset??? specified size between
18446744071562067969 and 18446744073709551615 exceeds maximum object
size 9223372036854775807 [-Werror=stringop-overflow=]
lto1: all warnings being treated as errors
lto-wrapper: fatal error: gcc returned 1 exit status
compilation terminated.
/usr/bin/ld: error: lto-wrapper failed
collect2: error: ld returned 1 exit status
child exited with value 1
make: *** [Makefile:132: w3m] Error 1
The function `unsigned char *MD5(const unsigned char *d, size_t n,
unsigned char *md)` from OpenSSL is deprecated and therefore generates
lots of warnings in the build when compiling against OpenSSL 3.0.
Do not treat these warnings as errors, but keep the warning active. This
should be reverted once OpenSSL-3.0 support arrives.
Let's at least ensure that the source builds without warnings.
The build image used, Ubuntu, spits out quite some warnings about
ignored return values. Use '-Wno-unused-result' for now.
The configure options are chosen to resemble the default Debian version.
With this patch applied the user can configure a directory to store
temporary/cache files. The history and cookies remain in RC_DIR.
I suppose the intent of writing the history to a temp file first is to
make the actual write an atomic operation. As rename() does not work
across mount points, we need to handle the temp file for the history
different to keep this behaviour.
Add a new type for the temp history file and handle this case different
when creating a temp file.
The if clause is true if cache is NULL. man 3 fclose says:
The behaviour of fclose() is undefined if the stream parameter is an
illegal pointer, or is a descriptor already passed to a previous invo‐
cation of fclose().
Check if cache is NULL before calling fclose().
This if is only true if both strchr return NULL, making sure that *q is
a NULL dereference.
The intention was to remove trailing '>' (or '\t') from the message-id
(p) for add_news_message().
Adjust the condition.
If, while adding the link number, we detect that we need to flush the
line, the anchor will be broken. When returning to the last breakpoint
only the link number will be removed, the internal anchor reference '<a
hseq=... >' remains on the flushed line.
Instead of adding the link number in a separate operation, check if we
just processed an anchor tag and prepend the current token with the link
number.
Throughout the whole code base only char * is passed, but a unsigned
char * is expected. This leads to several warnings.
Fix the interface and cast to unsigned char * internally to avoid any
changes to the behaviour.
This fixes issue #199 reported by Kuang-che Wu.
A specially crafted Gopher URL (e.g. '<a href=gopher:R>') could lead to
an out-of-bounds read.
Problem here was, that 'p' was incremented twice without checking for
the end of the string.
The interesting question for me is: What does this 'if' actually check?
What is special here about the 'R'? I did not find anything related in
RFC 1436 or in RFC 4266.
This patch fixes issue 200 'heap-buffer-overflow in
shiftAnchorPosition'.
Any input that looks like a tag, but isn't, should just be ignored. HTML
is parsed in a two staged process. Stage 1 inserts internal tags (like
<a hseq=...>). Stage 2 assumes these internal tags are valid input
inserted in the previous stage. If we don't throw away non-valid HTML
tags in stage 1 any website can inject tags that are interpreted as
internal ones. This leads to an out-of-bound read in this test case.
The 'hseq=-90' from the input ends as the value (89 actually) of
'a->hseq' in 'anchor.c:555'. Then 'hl->marks[a->hseq]' is out of bounds
as there are only 30 entries in this list.
This is the test input:
<table>0<br <>0<xmp>È«<div><inteRnal><input_alt fid=0><dl>0<dl>0<button value='">0000000000000000000000000000000000000000ÿ000'><A hseq=-90 href=>0<hR align=middle>
This is the same issue as commit 12c7b62a42 and should have been
included there.
'maxcol' can be -1 which results in a size of 0. The array is never
accessed with 'maxcol < 0', but zero length variable length arrays are
undefined behaviour.
Take the correct type from ASN1_STRING_get0_data() to fix a warning.
IA5Strings are ASCII, we do not need to care about signedness.
Since the pointer returned by ASN1_STRING_get0_data() points to internal
memory of the ASN1_STRING we are not allowed to manipulate the data.
Also it is not guaranteed that the data is null terminated. Copy the
data to our own buffer and make sure it is null terminated before using
it.
Compiling with GCC 10.2 with -fsanitize=address,undefined valgrind
and opening the 'opions panel' reports:
table.c:1632:8: runtime error: variable length array bound evaluates to non-positive value 0
table.c:1266:11: runtime error: variable length array bound evaluates to non-positive value 0
table.c:1267:12: runtime error: variable length array bound evaluates to non-positive value 0
'maxcell' is initialized to -1 which results in a size of 0 during the
first iteration. Though the array is only accessed if maxcell >= 0,
using a variable length array with a size < 1 is undefined behaviour
(see e.g. C99 6.7.5.2,p5).
This closes issue #51 .
Rene Kita