Package: nwipe
Version: 0.34-1+b1
Severity: normal
Tags: patch

I've used nwipe probably dozens of times on various times, and it
works fairly reliably. So I was surprised to find out it chokes on
this tiny little SSD drive here:

anarcat@angela:~$ sudo nwipe /dev/sdc
*** buffer overflow detected ***: terminated
Aborted
anarcat@angela:~[SIGABRT]$

Well isn't this odd! This is a fiarly normal drive:

anarcat@angela:~$ sudo smartctl -i -qnoserial /dev/sdc
smartctl 7.3 2022-02-28 r5338 [x86_64-linux-6.1.0-7-amd64] (local build)
Copyright (C) 2002-22, Bruce Allen, Christian Franke, www.smartmontools.org

=== START OF INFORMATION SECTION ===
Model Family:     Crucial/Micron RealSSD m4/C400/P400
Device Model:     M4-CT512M4SSD1
Firmware Version: 040H
User Capacity:    512 110 190 592 bytes [512 GB]
Sector Size:      512 bytes logical/physical
Rotation Rate:    Solid State Device
Form Factor:      2.5 inches
TRIM Command:     Available, deterministic
Device is:        In smartctl database 7.3/5319
ATA Version is:   ACS-2, ATA8-ACS T13/1699-D revision 6
SATA Version is:  SATA 3.0, 6.0 Gb/s (current: 6.0 Gb/s)
Local Time is:    Tue Apr 11 15:42:51 2023 EDT
SMART support is: Available - device has SMART capability.
SMART support is: Enabled

And speaking of smartctl, I suspect it's where nwipe chokes, because
as it turns out it crashes right after calling it, according to
strace:


anarcat@angela:~$ sudo strace -s8192 nwipe /dev/sdc
execve("/usr/sbin/nwipe", ["nwipe", "/dev/sdc"], 0x7ffda312d030 /* 15 vars */) 
= 0
brk(NULL)                               = 0x55afbf726000
mmap(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 
0x7fbb351ae000
[...]
read(3, "smartctl 7.3 2022-02-28 r5338 [x86_64-linux-6.1.0-7-amd64] (local 
build)\nCopyright (C) 2002-22, Bruce Allen, Christian Franke, 
www.smartmontools.org\n\n", 4096) = 150
read(3, "=== START OF INFORMATION SECTION ===\nModel Family:     Crucial/Micron 
RealSSD m4/C400/P400\nDevice Model:     M4-CT512M4SSD1\nSerial Number:    
REDACTED\nLU WWN Device Id: 5 00a075 109210beb\nFirmware Version: 040H\n", 
4096) = 223
read(3, "User Capacity:    512\342\200\257110\342\200\257190\342\200\257592 
bytes [512 GB]\nSector Size:      512 bytes logical/physical\nRotation Rate:    
Solid State Device\n", 4096) = 137
read(3, "Form Factor:      2.5 inches\nTRIM Command:     Available, 
deterministic\nDevice is:        In smartctl database 7.3/5319\nATA Version is: 
  ACS-2, ATA8-ACS T13/1699-D revision 6\nSATA Version is:  SATA 3.0, 6.0 Gb/s 
(current: 6.0 Gb/s)\nLocal Time is:    Tue Apr 11 15:44:34 2023 EDT\nSMART 
support is: Available - device has SMART capability.\nSMART support is: 
Enabled\n\n", 4096) = 366
read(3, "", 4096)                       = 0
--- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=72484, si_uid=0, 
si_status=0, si_utime=0, si_stime=0} ---
close(3)                                = 0
wait4(72484, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0, NULL) = 72484
writev(2, [{iov_base="*** ", iov_len=4}, {iov_base="buffer overflow detected", 
iov_len=24}, {iov_base=" ***: terminated\n", iov_len=17}], 3*** buffer overflow 
detected ***: terminated
) = 45
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 
0x7fbb351ad000
rt_sigprocmask(SIG_UNBLOCK, [ABRT], NULL, 8) = 0
gettid()                                = 72472
getpid()                                = 72472
tgkill(72472, 72472, SIGABRT)           = 0
--- SIGABRT {si_signo=SIGABRT, si_code=SI_TKILL, si_pid=72472, si_uid=0} ---
+++ killed by SIGABRT +++
Aborted

Of you look at the spaces in the "User Capacity" field more closely,
you'll notice it's actually a little odd. Those are not mere spaces in
those thousands separators, they are actually `U+202F NARROW NO-BREAK
SPACE` (represented by \342\200\257 in strace), according to
unicode(1). That, in itself, shouldn't be a problem: my terminal,
foot(1), is handling that fine, for example. But I bet it's the cause
of the overflow here.

Looking at gdb:

(gdb) bt
#0  __pthread_kill_implementation (threadid=<optimized out>, 
signo=signo@entry=6, 
    no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44
#1  0x00007ffff7d8dd2f in __pthread_kill_internal (signo=6, 
    threadid=<optimized out>) at ./nptl/pthread_kill.c:78
#2  0x00007ffff7d3eef2 in __GI_raise (sig=sig@entry=6)
    at ../sysdeps/posix/raise.c:26
#3  0x00007ffff7d29472 in __GI_abort () at ./stdlib/abort.c:79
#4  0x00007ffff7d822d0 in __libc_message (action=action@entry=do_abort, 
    fmt=fmt@entry=0x7ffff7e9c210 "*** %s ***: terminated\n")
    at ../sysdeps/posix/libc_fatal.c:155
#5  0x00007ffff7e1af32 in __GI___fortify_fail (
    msg=msg@entry=0x7ffff7e9c1b6 "buffer overflow detected")
    at ./debug/fortify_fail.c:26
#6  0x00007ffff7e19a40 in __GI___chk_fail () at ./debug/chk_fail.c:28
#7  0x00007ffff7e19322 in __strcpy_chk (dest=dest@entry=0x555555573622 "", 
    src=src@entry=0x7fffffffe850 "00000000125009210BEBUU", 
    destlen=destlen@entry=21) at ./debug/strcpy_chk.c:30
#8  0x000055555555ff22 in strcpy (__src=0x7fffffffe850 
"00000000125009210BEBUU", 
    __dest=0x555555573622 "")
    at /usr/include/x86_64-linux-gnu/bits/string_fortified.h:79
#9  check_device (c=c@entry=0x7fffffffe918, dev=<optimized out>, 
    dcount=dcount@entry=0) at ./src/device.c:271
#10 0x000055555556010b in nwipe_device_get (c=c@entry=0x7fffffffe918, 
    devnamelist=<optimized out>, ndevnames=ndevnames@entry=1)
    at ./src/device.c:114
#11 0x0000555555557823 in main (argc=1, argv=<optimized out>) at 
./src/nwipe.c:148
(gdb) 

Turns out that is the serial number of the drive, not the size. Who
would have thought...

And indeed, the serial number is 22 characters long, while nwipe only
accepts 20-character long passwords.

https://sources.debian.org/src/nwipe/0.34-1/src/context.h/#L104

Interestingly, this is caught by fortify, so it's hopefully not
exploitable, at least not in Debian as it is.

At the very least this should be a strncpy which would fail to copy
the entire serial number, but not completely crash. Arguably, the
length of the serial number could be bumped as well, but that probably
beyond the scope of this.

Really, who uses strcpy in this day and age anyway?

A workaround *should* to use the --quiet flag, but nwipe *still*
attempts to copy the serial number in that case, ugh.

The attached patch should fix this issue.

-- System Information:
Debian Release: 12.0
  APT prefers testing-security
  APT policy: (500, 'testing-security'), (500, 'stable-security'), (500, 
'testing'), (500, 'stable'), (1, 'experimental'), (1, 'unstable')
Architecture: amd64 (x86_64)

Kernel: Linux 6.1.0-7-amd64 (SMP w/16 CPU threads; PREEMPT)
Locale: LANG=fr_CA.UTF-8, LC_CTYPE=fr_CA.UTF-8 (charmap=UTF-8), LANGUAGE not set
Shell: /bin/sh linked to /usr/bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled

Versions of packages nwipe depends on:
ii  libc6        2.36-8
ii  libncurses6  6.4-2
ii  libparted2   3.5-3
ii  libtinfo6    6.4-2

nwipe recommends no packages.

nwipe suggests no packages.

-- no debconf information
Description: use strncpy to extra serial number

 The serial number here comes from an untrusted source and can have an
 arbitrary length. I have just found an example in a wild that has 22
 characters, which crashes nwipe.
 
Author: Antoine Beaupré <anar...@debian.org>
Bug-Debian: https://bugs.debian.org/<bugnumber>
Forwarded: no
Last-Update: 2023-04-11

--- nwipe-0.34.orig/src/device.c
+++ nwipe-0.34/src/device.c
@@ -268,7 +268,7 @@ int check_device( nwipe_context_t*** c,
         /* If the serial number hasn't already been populated */
         if( next_device->device_serial_no[0] == 0 )
         {
-            strcpy( next_device->device_serial_no, tmp_serial );
+            strncpy( next_device->device_serial_no, tmp_serial, 20 );
         }
     }
 

Reply via email to