4 ms·
p = (char *)(path + strlen(path) - 4); This is a very bad idea as path is user-supplied and has to be treated as malicious. An attacker can omit the string-ter
by KonradKlause 15y ago
p = (char *)(path + strlen(path) - 4);
This is a very bad idea as path is user-supplied and has to be treated as malicious.
An attacker can omit the string-terminator...
- muppetman 15y agoYou're inserting a dodgy bit of a code into a live running kernel. You are an attacker! I mean, valid technicality, but really? If you've got the ability to insert any module you want, you have the ability (ok, not always, but usually) to rm -rf /
- KonradKlause 15y agoBullshit. Anyone who's using open() can kill the kernel after loading this module.
- burgerbrain 15y agoProbably best that any user can kill the kernel in this situation, if you stop to think about it.
- deleted 15y ago[deleted]
- muppetman 15y agoAnyone who can load a kernel module can do _whatever they want_. That was my point, which you've obviously missed.
- KonradKlause 15y agoAnd you are obviously missing that modern operating systems have more than just one user. :-)
- muppetman 15y agoI'm genuinely interested - what do you mean? It's my understanding (and please, correct me if I'm wrong) that if you're able to insmod a non-standard kernel module into a linux system, you are root already. I don't understand what this has to do with unix being multi-user, only root has the ability to insmod /tmp/haha.ko Please note I'm not talking about the ability of the kernel to automatically load modules as required. That's only from the /lib/modules path and you need to be root have to inserted a module in there anyway. And if you have that ability, worrying about the security of a module such as this seems, to me, totally and utterly pointless, because you could be inserting a rootkit, or a module that randomly deletes a file every time you press the letter A etc. So could you explain what you mean please? Why be concerned about security when security is already compromised? How does it being multi-user make a difference?
- KonradKlause 15y agoSure. Suppose your admin is a funny guy and loads this kernel module at April 1st. I (Jonny non-root user) can no longer listen to my favorite mp3, because the kernel module changes the path all the time. The admin loughs and has fun. But then later I call open() with some fancy arguments and take over the whole machine (or at least crash it). This is the part where I lough.
- muppetman 15y agoAhhh right, yes, Ok, I see what you mean. If you loaded this on a serious system, you're insane. But right, I see your point.
- dexen 15y agoI guess you meant, ``Anyone who's using open() can kill the kernel after somebody else loads the module.''. Which is far worse: any unprivileged user, say the victim of rickroll, can bite back hard.
- gfodor 15y agoThis seems like a feature not a bug.
- KonradKlause 15y agoBTW: write_cr0(read_cr0() | 0x10000) is a absolute no-go. It's a nice and funny kernel module but it dooms the kernel's security.
- exDM69 15y agoFor the uninitiated, cr0 is a control register in the x86 family of processors. Bit 16 of that register is (according to wikipedia) WP - Write protect: Determines whether the CPU can write to pages marked read-only. What this means is that the kernel can no longer see if a user is writing to a read-only location, which is a major part of memory protection. It has major security implications. I don't know whether other parts of the kernel enable or disable this at context switches or at other times. What this also means is that this kernel module will only work on x86 CPU's. So no rickrolling you ARM boxes or smartphones.
- bstx 15y ago> What this means is that the kernel can no longer see if a user is writing to a read-only location, which is a major part of memory protection. No, disabling WP in CR0 is only relevant for privileged code. Writes to write protected pages in user mode will still trap. It might mess up some in-kernel COW stuff though.
- adobriyan 15y agoIf we could take CR0 et al manipulations away from module authors, we'd do it long ago. Sigh...
- jrockway 15y agoThis is why I avoid out-of-tree modules. At least someone had to glance over the in-tree module before I run it on my machine. When you download random kernel modules from the Internet, they're probably from people too lazy to have their code properly reviewed. And if they're too lazy to have their code reviewed, they probably don't feel too bad about disabling all page write protection, either.
- 15y ago
- deleted 15y ago[deleted]
- merijnv 15y agoEh, the code does look for the string terminator. It calls strlen, which indexes the char* until it finds a NULL byte. If the string doesn't have a NULL byte then strlen will happily continue reading past the boundary until it either segfaults or finds a NULL byte.
- forgotusername 15y agoJust some random nitpickery, but "NULL" has a specific meaning in C, which is 0 cast to a void pointer. So preferably talk about a "NUL" (as it appears in most ASCII tables) or perhaps "null character" (which is what recent C standards prefer instead, cant remember why they changed though).
- datenwolf 15y agoAnd… you got your nitpickery wrong. A C null pointer is just 0. The exact definition in the statement is "a null pointer is a integral constant expression with value 0". There are some stddef.h which cast it to a void*, but technically that's just a compatibility hack. See http://c-faq.com/null/macro.html http://c-faq.com/null/macro.html http://c-faq.com/null/safermacs.html http://c-faq.com/null/safermacs.html http://c-faq.com/null/nullor0.html http://c-faq.com/null/nullor0.html http://c-faq.com/null/long0.html http://c-faq.com/null/long0.html
- exDM69 15y agoOut of curiosity, I checked out how the open syscall in linux works (see fs/open.c). It uses getname (from fs/namei.h), which effectively copies the filename from userland memory to a chunk of kernel memory. It uses strncpy_from_user to do this and uses PATH_MAX as the maximum length. So even the linux kernel does rely on the fact that there is a null terminator in the string. So your assumption on omitting the null terminator is wrong, BUT there's another flaw in this. The length of the string can be less than four bytes. By allocating the string in a nice page boundary this can be used to cause a memory protection fault.
- KonradKlause 15y ago> It uses strncpy_from_user to do this and uses PATH_MAX as the maximum length. So even the linux kernel does rely on the fact that there is a null terminator in the string. But the kernel does not call strlen() on the user-supplied string...
- 0x12 15y ago> It uses strncpy_from_user to do this and uses PATH_MAX as the maximum length. So even the linux kernel does rely on the fact that there is a null terminator in the string. The PATH_MAX guarantees that the string copy will terminate even if no null character is present. Now if any routines that call strncpy_from_user assume that it leaves them with a null terminated string there is still a potential for trouble but that does not stem from the use of strncpy_from_user. Check the implementation for do_getname: http://lxr.free-electrons.com/source/fs/namei.c http://lxr.free-electrons.com/source/fs/namei.c And you'll see that it returns -ENAMETOOLONG when the name is >= the buffer allocated, only < len is allowed. (line 133 in the link above).