Iljas Blag

Every once in a while I stumble on some kernel code where the code attempts to do authentication based on the pid of the calling process.
This does not work ! it’s insecure, just don’t do it!
Usually, the code assumes that process with pid x has certain privilege’s.
for example, lets say you only want root to issue ioctl’s on a device.
you’d make the open callback for your device do something like:
open_fn() {
if (current->uid == 0)
add_pid_to_trusted_list(current->pid);
}

and then for all your ioctl’s you’d just check if current->pid is in the trusted list.

This is a horrible kludge !!
All of this works fine, until the procces that opened the device unexpectedly dies.
now there is a dangling pid in the trusted list. all an attacker would have to do is spawn off new processes until you end up with the pid that’s in the trusted list.
and _BAM_ the attacker gets to issues ioctl’s on a device he really shouldn’t get to issues ioctl’s on.

don’t think apps do this ? let’s look at BestCrypt (http://www.jetico.com/).
here’s it’s open callback:
static int bc_open(struct inode *inode, struct file *file)
{
…
if (capable(CAP_SYS_ADMIN)) {
bc_add_pid(current->pid);
}
return 0;
}

it’s ioctl handler looks like:
static int bc_ioctl(struct inode *inode, struct file *file, u_int cmd, u_long arg)
{
…
switch (cmd) {

BC_HANDLER(”get_info”, BC_GET_INFO, bc_get_info(bc, (struct bc_info*) arg));
BC_HANDLER(”set_fd “, BC_SET_FD, bc_set_fd (bc, bdev, (struct bc_file64 *) arg));
BC_HANDLER(”clr_fd “, BC_CLR_FD, bc_clr_fd (bc, bdev, inode));
BC_HANDLER(”lock_dev”, BC_LOCK_DEV, bc_lock_dev(bc, inode->i_rdev, 1));
BC_HANDLER(”ulck_dev”, BC_UNLOCK_DEV, bc_lock_dev(bc, inode->i_rdev, 0));
BC_HANDLER(”frc_ulck”, BC_FORCE_UNLOCK, bc_force_unlock(bc, bdev, inode->i_rdev));
BC_HANDLER(”get_priv”, BC_GET_PRIV, bc_get_priv(arg));
BC_HANDLER(”hdio_geo”, HDIO_GETGEO, hdio_getgeo(bc, (struct hd_geometry *) arg));
BC_HANDLER(”vrfy_alg”, BC_VERIFY_ALG, bc_vrfy_alg((struct bc_alg*) arg));
BC_HANDLER(”make_key”, BC_MAKE_KEY, bc_make_key((struct bc_key*) arg));
BC_HANDLER(”free_key”, BC_FREE_KEY, bc_free_key((struct bc_key*) arg));
BC_HANDLER(”encr_blk”, BC_ENCRYPT_BLOCK, bc_process ((struct bc_block*) arg, BC_ENCRYPT_BLOCK));
BC_HANDLER(”decr_blk”, BC_DECRYPT_BLOCK, bc_process ((struct bc_block*) arg, BC_DECRYPT_BLOCK));
}
…
}

BC_HANDLER is an ugly macro that looks like:
#define BC_HANDLER(dbg, x, y) case (x): /*printk(dbg “\n”); */\
error = (y); \
break;
all of the functions used there looks like:
static int some_function(struct bc_device *bc, struct block_device *bdev, struct bc_file64 *arg) {
… variable declaration …
if (bc_find_pid_safe(current->pid) pid) >= 0) {
current->cap_effective |= (1<<CAP_SYS_ADMIN)|(1<<CAP_CHOWN)|(1<<CAP_DAC_OVERRIDE)|(1<euid = 0;
} else {
return -EPERM;
}

if (arg)
bc_del_pid(current->pid);
return 0;
}

yea, isn’t that great ?
Oh, and here’s the kicker, BestCrypt comes with an suid root application that’ll open the device for you ! (which means you can just kill it once it’s opened the device).
These kind of fuckup’s don’t limit themself to linux. I’ve seen similar screwups in windows drivers.