Bug#1034262: buffer overflow on peculiar drive

[Martijn van Brummelen]

Hi Antoine,

Thank for this patch , will upload/fix it soon after I checked if the
length is still sufficient or not.

Kind regards,
Martijn van Brummelen
On 2023-04-11 22:20, Antoine Beaupre wrote:
> 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