Re: [PATCH] volume_id: volume_id_get_buffer with small FSes
On Sun, 15 May 2022 17:18:47 + Alyssa Ross wrote: > I tested both the cases mentioned in the comments — an empty floppy > drive, and an unused loop device. Reading from an empty floppy drive > is ENXIO, and reading from an unused loop device returns 0, so it > should be fine to drop the minimum length, and be happy with any read > as long as it returns at least one byte. > - * accesses. Rationale: > - * users complained of slow blkid due to empty floppy drives. But is it slow like the comment said? Ie. does this patch cause a 1-2s delay due to the floppy drive spinning up? - Lauri ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCHv2]ash: Add ifsfree to varunset and varvalue function to fix a logic error that leaks the heap
Details:
First bug:
Due to a logic error in the ifsbreakup function in ash.c when a
heredoc and normal command is run one after the other by means of a
semi-colon, when the second command drops into ifsbreakup the command
will be evaluated with the ifslastp/ifsfirst struct that was set when
the here doc was evaluated. This results in a buffer over-read that
can leak the program's heap, stack, and arena addresses which can be
used to beat ASLR.
Second bug:
If the heap is sprayed with a certain amount of bash variables and
part of the first bug is sent, a predictable heap value can be free'd
and put into the tcache. After the heap value is free'd if the heap
was sprayed correctly, an attacker can overwrite the free’d tcache
address to obtain numerous write-what-where and the ability to
arbitrarily overwrite any writable memory address. This could lead to
a DoS, arbitrary code execution, or possible privilege escalation if
the setuid flag is set.
Steps to Reproduce:
First bug:
cmd args: ~/exampleDir/example> busybox ash
$ M='A'
$ q00(){
$ <<000;echo
$ ${D?$M$M$M$M$M$M}
$ 000
$ }
$ q00
Second bug:
cmd args: ~/exampleDir/example> busybox ash
$ AA
$ `spray 600 bash variables with size of 0x30 bytes`
$ `send bash variable with size of 0x20 bytes`
$ `send bash variable with size of 0x60 bytes`
$ `spray 12 bash variables with size of 0x20 bytes`
$ `Send part of first vulnerability`
$ <<00;V
$ ${x?0p$^?A<$B*442>$0bdbasdfg$0}
in this line are not meant to be entered in as is, but
instead shows amount of letter inside <> that would be entered in.>
$ 00
Patch:
Adding the following to ash.c will fix both bugs in one go.
--
--- a/shell/ash.c
+++ b/shell/ash.c
@@ -7030,6 +7030,7 @@
msg = umsg;
}
}
+ifsfree();
ash_msg_and_raise_error("%.*s: %s%s", (int)(end - var - 1), var, msg, tail);
}
@@ -7445,6 +7446,7 @@
if (discard)
return -1;
+ifsfree();
raise_error_syntax("bad substitution");
}
--
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] volume_id: volume_id_get_buffer with small FSes
I was working with some very small ext4 filesystems, used for
overlaying on immutable VM/container images. If it's only desired to
overlay a single config file, these filesystems can be very small
indeed, and so ran afoul of this length check.
I tested both the cases mentioned in the comments — an empty floppy
drive, and an unused loop device. Reading from an empty floppy drive
is ENXIO, and reading from an unused loop device returns 0, so it
should be fine to drop the minimum length, and be happy with any read
as long as it returns at least one byte.
By initializing read_len to -1, we can handle both lseek() failing and
read_full() failing with the same check.
---
util-linux/volume_id/util.c | 10 ++
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/util-linux/volume_id/util.c b/util-linux/volume_id/util.c
index 061545fde..a5844c673 100644
--- a/util-linux/volume_id/util.c
+++ b/util-linux/volume_id/util.c
@@ -180,7 +180,7 @@ void *volume_id_get_buffer(struct volume_id *id, uint64_t
off, size_t len)
{
uint8_t *dst;
unsigned small_off;
- ssize_t read_len;
+ ssize_t read_len = -1;
dbg("get buffer off 0x%llx(%llu), len 0x%zx",
(unsigned long long) off, (unsigned long long) off, len);
@@ -237,13 +237,7 @@ void *volume_id_get_buffer(struct volume_id *id, uint64_t
off, size_t len)
dbg("requested 0x%x bytes, got 0x%x bytes",
(unsigned) len, (unsigned) read_len);
err:
- /* No filesystem can be this tiny. It's most likely
-* non-associated loop device, empty drive and so on.
-* Flag it, making it possible to short circuit future
-* accesses. Rationale:
-* users complained of slow blkid due to empty floppy drives.
-*/
- if (off < 64*1024)
+ if (read_len <= 0)
id->error = 1;
/* id->seekbuf_len or id->sbbuf_len is wrong now! Fixing. */
volume_id_free_buffer(id);
--
2.35.1
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
