tats / w3m

Debian's w3m: WWW browsable pager
https://tracker.debian.org/pkg/w3m
Other
852 stars 92 forks source link

Incomplete fix for CVE-2022-38223 #268

Closed iskindar closed 1 year ago

iskindar commented 1 year ago

Hi, I think the fix for CVE-2022-38223 in 419ca82 is not complete and it is still possible to trigger the same bug with a different poc in tats/w3m#242. The bug is an out of bound write in checkType, etc.c:478.

Version

w3m latest commit 93ad5ee

./w3m --version
w3m version w3m/0.5.3+git20230129, options lang=en,m17n,image,color,ansi-color,mouse,menu,cookie,external-uri-loader,w3mmailer,nntp,gopher,ipv6,alarm,mark

How to reproduce

export CC="gcc -fsanitize=address -g"  && ./configure && make -j
./w3m -dump $POC

ubuntu 20.04 dockerized reproduce steps

docker pull ubuntu:20.04 && docker run -it ubuntu:20.04 bash
## now step into the container
apt update && apt install wget git unzip gcc g++ make libgc-dev libtinfo-dev -y
git clone https://github.com/tats/w3m && pushd w3m
export CC="gcc -fsanitize=address -g" && ./configure && make -j
wget https://github.com/tats/w3m/files/11966800/poc0.zip && unzip poc0.zip
./w3m -dump ./poc0

Debian 11 dockerized reproduce steps

docker pull debian:11 && docker run -it debian:11 bash
## now step into the container
apt update && apt install wget git unzip gcc g++ make libgc-dev libtinfo-dev -y
git clone https://github.com/tats/w3m && pushd w3m
export CC="gcc -fsanitize=address -g" && ./configure && make -j
wget https://github.com/tats/w3m/files/11967522/poc0_debian.zip && unzip poc0_debian.zip
./w3m -dump ./poc0

ASAN log

AddressSanitizer:DEADLYSIGNAL
=================================================================
==5589==ERROR: AddressSanitizer: SEGV on unknown address 0x55e29237fe3c (pc 0x55e37f25f7d5 bp 0x55e29237fe3c sp 0x7fffda0d3220 T0)
==5589==The signal is caused by a WRITE memory access.
    #0 0x55e37f25f7d4 in checkType /benchmark/w3m/etc.c:478
    #1 0x55e37f225e8f in loadBuffer /benchmark/w3m/file.c:7727
    #2 0x55e37f24ac5b in loadSomething /benchmark/w3m/file.c:232
    #3 0x55e37f24ac5b in loadGeneralFile /benchmark/w3m/file.c:2288
    #4 0x55e37f1e8807 in main /benchmark/w3m/main.c:1061
    #5 0x7f216f6dd082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082)
    #6 0x55e37f1ec56d in _start (/benchmark/w3m/w3m+0xb256d)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /benchmark/w3m/etc.c:478 in checkType
==5589==ABORTING

Platform

The bug was found by my fuzzer on Ubuntu 20.04.5. In addition, the bug can also be reproduced on Debian 11 with the default version of gcc.

PoC

ubuntu poc0.zip debian poc0.zip

PS: The poc is different from that of tats/w3m#242 .

iskindar commented 1 year ago

Affected version :

Not Affected version: < 0.5.3+git20220429-1

rkta commented 1 year ago

Introduced in

commit 419ca82d57c72242817b55e2eaa4cdbf6916e7fa (HEAD, refs/bisect/bad) Author: Tatsuya Kinoshita @.***> Date: 2022-12-20T21:16:48+09:00

Fix m17n backspace handling causes out-of-bounds write in checkType

[CVE-2022-38223]
Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1019599
Bug-Debian: https://github.com/tats/w3m/issues/242

In function checkType in line 386 plens pointer goes out of bounds. Garbage is assigned to plen, this garbage is subtracted from prop and prop is later derefenced which leads to the SIGSEV.

(The poc file is full of backspaces.)

rkta commented 1 year ago

From how I understand the code, plen is the length of the current (or previous?) character. In order to process backspace chars correctly over multi-char characters we store the length of each character in plens_buffer. When reaching a backspace we pop the last length from the array.

Now, if we get more backspace chars than we have characters processed the pointer will go out of bounds.

Below is a fix which prevents the pointer from going out of bounds and setting plen to 0 when there is nothing left to process.

This also fixes [BUG] Out of bound read in Strnew_size , Str.c:61 #270

diff --git a/etc.c b/etc.c index 128717b1..efcfab92 100644 --- a/etc.c +++ b/etc.c @@ -393,7 +393,8 @@ checkType(Str s, Lineprop oprop, Linecolor ocolor) if (color) color -= plen;

endif

tats commented 1 year ago

@rkta Could you please create a pull request?

rkta commented 1 year ago

On Wed, Jul 12, 2023 at 06:59:48AM -0700, Tatsuya Kinoshita wrote:

@rkta Could you please create a pull request?

See https://github.com/tats/w3m/pull/273

tats commented 1 year ago

Fixed with https://github.com/tats/w3m/pull/273

carnil commented 8 months ago

This has recieved a separate CVE: CVE-2023-4255

rkta commented 8 months ago

On Fri, Dec 22, 2023 at 08:21:39AM -0800, Salvatore Bonaccorso wrote:

This has recieved a separate CVE: CVE-2023-4255

Did you mean to add this to #282? It's not sure to me what the new CVE is about.