Hadley Climate Research Unit at University of East Anglia in Norwich was hacked and 61MB of confidential data were released to public. Here is where I read about that.
CVE-2009-3909: GIMP PSD Heap Based Buffer Overflow
•November 20, 2009 • Leave a CommentThis vulnerability was discovered by Secunia Research and it affects GIMP 2.6.7 and probably earlier releases too. The buggy code resides in the plug-in that is responsible for loading PSD image files. Here is this code as seen in plug-ins/file-psd/psd-load.c.
static gint
read_channel_data (PSDchannel *channel,
const guint16 bps,
const guint16 compression,
const guint16 *rle_pack_len,
FILE *f,
GError **error)
{
gchar *raw_data;
gchar *src;
gchar *dst;
guint32 readline_len;
gint i;
...
IFDBG(3) g_debug ("raw data size %d x %d = %d", readline_len,
channel->rows, readline_len * channel->rows);
raw_data = g_malloc (readline_len * channel->rows);
switch (compression)
{
case PSD_COMP_RAW:
...
memcpy (raw_data + i * readline_len, dst, readline_len);
g_free (dst);
}
break;
}
...
return 1;
}
The above routine is used to read the channel information of the PSD file. However, the size calculation for the length of each line, represented by ‘readline_len’ mupliplied by the number of rows (through ‘channel->rows’ variable), can result in an integer overflow and allocation of an incorrect size of heap space using g_malloc(). The subsequent operations will eventually result in heap memory corruption.
This was fixed using the patch below.
IFDBG(3) g_debug ("raw data size %d x %d = %d", readline_len,
channel->rows, readline_len * channel->rows);;
+
+ /* sanity check, int overflow check (avoid divisions by zero) */
+ if ((channel->rows == 0) || (channel->columns == 0) ||
+ (channel->rows > G_MAXINT32 / channel->columns / MAX (bps >> 3, 1)))
+ {
+ g_set_error (error, G_FILE_ERROR, G_FILE_ERROR_FAILED,
+ _("Unsupported or invalid channel size"));
+ return -1;
+ }
+
raw_data = g_malloc (readline_len * channel->rows);
Where they check the channel size before proceeding with the actual allocation. For completeness, here is the definition of the ‘PSDchannel’ type as seen in plug-ins/file-psd/psd.h:
/* PSD Channel data structure */
typedef struct
{
gint16 id; /* Channel ID */
gchar *name; /* Channel name */
gchar *data; /* Channel image data */
guint32 rows; /* Channel rows */
guint32 columns; /* Channel columns */
} PSDchannel;
Linux kernel FUSE Uninitialized Pointer Access
•November 19, 2009 • Leave a CommentI saw this vulnerability on oss-security mailing list, reported by Eugene Teo of Red Hat. As he says, the bug affects Linux kernel between versions 2.6.14-rc1 and 2.6.32-rc7. The bug however, was discovered and reported by Anand V. Avati. If we have a look at fs/fuse/file.c of 2.6.31 release of the Linux kernel, we’ll see the following.
ssize_t fuse_direct_io(struct file *file, const char __user *buf,
size_t count, loff_t *ppos, int write)
{
...
struct fuse_req *req;
...
while (count) {
...
int err = fuse_get_user_pages(req, buf, &nbytes, write);
if (err) {
res = err;
break;
}
...
fuse_release_user_pages(req, !write);
if (req->out.h.error) {
...
} else if (nres > nbytes) {
res = -EIO;
break;
...
if (nres != nbytes)
break;
...
req = fuse_get_req(fc);
if (IS_ERR(req))
break;
}
}
fuse_put_request(fc, req);
if (res > 0)
*ppos = pos;
return res;
}
The above routine is used by the FUSE filesystem to allocate the requests to the client represented by a ‘fuse_req’ structure which is defined at fs/fuse/fuse_i.h. Anand V. Avati noticed that when breaking out of the allocation loop because of an error condition, fuse_put_request() will be invoked in a probably invalid pointer since request could contain an invalid pointer and that also could be the reason of breaking out of the ‘while’ loop.
This was fixed by adding an error check for the ‘req’ pointer like this:
}
}
- fuse_put_request(fc, req);
+ if (!IS_ERR(req))
+ fuse_put_request(fc, req);
if (res > 0)
*ppos = pos;
Additionally, Eugene Teo discussed a code path used to trigger the bug. Specifically, he says that the call to fuse_get_req() inside fuse_direct_io() shown above will lead to execution of the following:
struct fuse_req *fuse_get_req(struct fuse_conn *fc)
{
struct fuse_req *req;
...
atomic_inc(&fc->num_waiting);
...
req = fuse_request_alloc();
err = -ENOMEM;
if (!req)
goto out;
...
out:
atomic_dec(&fc->num_waiting);
return ERR_PTR(err);
}
Function fuse_request_alloc() is using kmem_cache_alloc() to initialize ‘req’ pointer but under certain circumstances, it could fail to allocate the required space and return the ‘req’ pointer uninitialized. If this is the case, fuse_get_req() will fall to the ‘out’ label that will return will error.
Back to fuse_direct_io() we can read that if the call to fuse_get_req() fails, it will break out of the loop and call fuse_put_request() on the uninitialized/invalid pointer ‘req’. This will lead to the following routine which can be found at fs/fuse/dev.c:
void fuse_put_request(struct fuse_conn *fc, struct fuse_req *req)
{
if (atomic_dec_and_test(&req->count)) {
if (req->waiting)
atomic_dec(&fc->num_waiting);
if (req->stolen_file)
put_reserved_req(fc, req);
else
fuse_request_free(req);
}
}
EXPORT_SYMBOL_GPL(fuse_put_request);
The first call to atomic_dec_and_test() will lead to an invalid pointer access when it’ll attempt to decrement the ‘count’ member of that pointer which would be located in something like ‘&(uninitialized_ptr)->count’. The same would also happen in the ‘req->stolen_file’ check that will call either put_reserved_req() or fuse_request_free().
The first case of calling put_reserved_req() will lead to a possibly exploitable situation because of all these operations on the invalid pointer:
/*
* Put stolen request back into fuse_file->reserved_req
*/
static void put_reserved_req(struct fuse_conn *fc, struct fuse_req *req)
{
struct file *file = req->stolen_file;
struct fuse_file *ff = file->private_data;
spin_lock(&fc->lock);
fuse_request_init(req);
BUG_ON(ff->reserved_req);
ff->reserved_req = req;
wake_up_all(&fc->reserved_req_waitq);
spin_unlock(&fc->lock);
fput(file);
}
Whereas, the latter scenario, it leads to an already known to be exploitable call to this:
void fuse_request_free(struct fuse_req *req)
{
kmem_cache_free(fuse_req_cachep, req);
}
Interesting vulnerability although it will require some clever way to make the kmem_cache_alloc() fail and predict the location of the uninitialized pointer in order to have a controlled cache de-allocation that can lead to code execution in the kernel context.
CVE-2009-3080: Linux kernel GDT ISA/EISA/PCI Disk Array Controllers Signedness Issue
•November 19, 2009 • Leave a CommentThis bug was discovered and reported by Dave Jones of Red Hat. The susceptible code resides in drivers/scsi/gdth.c which has the device driver code for ICP vortex GmbH. GDT ISA/EISA/PCI disk array controllers and Intel storage RAID controllers. Here is the buggy routine as seen in 2.6.31 release of the Linux kernel.
static int gdth_read_event(gdth_ha_str *ha, int handle, gdth_evt_str *estr)
{
gdth_evt_str *e;
int eindex;
ulong flags;
TRACE2(("gdth_read_event() handle %d\n", handle));
spin_lock_irqsave(&ha->smp_lock, flags);
if (handle == -1)
eindex = eoldidx;
else
eindex = handle;
estr->event_source = 0;
if (eindex >= MAX_EVENTS) {
spin_unlock_irqrestore(&ha->smp_lock, flags);
return eindex;
}
e = &ebuffer[eindex];
if (e->event_source != 0) {
if (eindex != elastidx) {
if (++eindex == MAX_EVENTS)
eindex = 0;
} else {
eindex = -1;
}
memcpy(estr, e, sizeof(gdth_evt_str));
}
spin_unlock_irqrestore(&ha->smp_lock, flags);
return eindex;
}
As it is implied by its name, this function is used to read the event passed through its third argument that is a pointer to a ‘gdth_evt_str’ which as you can read in drivers/scsi/gdth_ioctl.h is a type definition for a structure that describes the event.
typedef struct {
ulong32 first_stamp;
ulong32 last_stamp;
ushort same_count;
ushort event_source;
ushort event_idx;
unchar application;
unchar reserved;
gdth_evt_data event_data;
} PACKED gdth_evt_str;
Back to gdth_read_event() we can read that ‘eindex’ is initialized with the old index value if the handle passed to that routine by its first argument is -1, otherwise it will be initialized with handle’s value. The next check against ‘MAX_EVENTS’, which is the maximum buffer events and it is declared as 100 in drivers/scsi/gdth.h, can be bypassed since ‘eindex’ is a singed integer and it is perfectably legal to contain negative values which are obviously less than 100 and can also be something else instead of -1.
The subsequent operations that use that value as an index to events’ buffer ‘ebuffer[]‘ as well as the final memcpy() from the possibly invalid ‘e’ pointer can result in a memory corruption situation.
This vulnerability was fixed using the patch below.
estr->event_source = 0;
- if (eindex >= MAX_EVENTS) {
+ if (eindex < 0 || eindex >= MAX_EVENTS) {
spin_unlock_irqrestore(&ha->smp_lock, flags);
return eindex;
In order to check for negative values too.
So, before I end this post have a look at the following function which you’ll probably find useful…
static int ioc_event(void __user *arg)
{
gdth_ioctl_event evt;
gdth_ha_str *ha;
ulong flags;
if (copy_from_user(&evt, arg, sizeof(gdth_ioctl_event)))
...
if (evt.erase == 0xff) {
...
} else if (evt.erase == 0) {
evt.handle = gdth_read_event(ha, evt.handle, &evt.event);
...
if (copy_to_user(arg, &evt, sizeof(gdth_ioctl_event)))
return -EFAULT;
return 0;
}
Linux kernel KVM Memory Corruption on MCE setup
•November 17, 2009 • Leave a CommentI saw this vulnerability in securityfocus, the bug was reported by Jan Kiszka of Siemens as we can read from 2.6.32-rc7’s ChangeLog file.
Here is the buggy code as seen in 2.6.32-rc6 release of the Linux kernel…
static int kvm_vcpu_ioctl_x86_setup_mce(struct kvm_vcpu *vcpu,
u64 mcg_cap)
{
int r;
unsigned bank_num = mcg_cap & 0xff, bank;
r = -EINVAL;
if (!bank_num)
goto out;
if (mcg_cap & ~(KVM_MCE_CAP_SUPPORTED | 0xff | 0xff0000))
goto out;
r = 0;
vcpu->arch.mcg_cap = mcg_cap;
/* Init IA32_MCG_CTL to all 1s */
if (mcg_cap & MCG_CTL_P)
vcpu->arch.mcg_ctl = ~(u64)0;
/* Init IA32_MCi_CTL to all 1s */
for (bank = 0; bank < bank_num; bank++)
vcpu->arch.mce_banks[bank*4] = ~(u64)0;
out:
return r;
}
The above routine was ripped from arch/x86/kvm/x86.c file, and it is used to setup MCE (Machine Check Exception) support in the virtual processor, this is a recently added feature in KVM as we can find here. Anyway, the above code assumes that the user controlled MCE banks’ value (stored in ‘bank_num’) will not be more than 32 (which is declared by the ‘KVM_MAX_MCE_BANKS’ constant) but on the other hand, it allows a user to specify up to 255 MCE banks because of the:
unsigned bank_num = mcg_cap & 0xff, bank;
That masks it with 0xFF and doesn’t perform any range checks after doing so. The subsequent ‘for’ loop in kvm_vcpu_ioctl_x86_setup_mce() will result in kernel memory corruption since it will attempt to copy data beyond ‘vcpu->arch.mce_banks[]‘ array’s bounds. This array has a static size which you can find in kvm_arch_vcpu_init() function.
int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
{
struct page *page;
struct kvm *kvm;
int r;
BUG_ON(vcpu->kvm == NULL);
kvm = vcpu->kvm;
...
vcpu->arch.mce_banks = kzalloc(KVM_MAX_MCE_BANKS * sizeof(u64) * 4,
GFP_KERNEL);
...
return r;
}
Of course, the patch was to add that missing check like this:
unsigned bank_num = mcg_cap & 0xff, bank;
r = -EINVAL;
- if (!bank_num)
+ if (!bank_num || bank_num >= KVM_MAX_MCE_BANKS)
goto out;
if (mcg_cap & ~(KVM_MCE_CAP_SUPPORTED | 0xff | 0xff0000))
Finally, this code can be reached through ‘KVM_X86_SETUP_MCE’ IOCTL command, here is the exact code path in the IOCTL handler.
long kvm_arch_vcpu_ioctl(struct file *filp,
unsigned int ioctl, unsigned long arg)
{
struct kvm_vcpu *vcpu = filp->private_data;
void __user *argp = (void __user *)arg;
int r;
struct kvm_lapic_state *lapic = NULL;
switch (ioctl) {
...
case KVM_X86_SETUP_MCE: {
u64 mcg_cap;
r = -EFAULT;
if (copy_from_user(&mcg_cap, argp, sizeof mcg_cap))
goto out;
r = kvm_vcpu_ioctl_x86_setup_mce(vcpu, mcg_cap);
break;
}
...
return r;
}
News: David Irving got Owned
•November 16, 2009 • Leave a CommentI just read this post on wired.
Additionally, here are two wikileaks links:
- the first one
- the second one
Sun xVM VirtualBox Guest Additions Memory Consumption (on guest OS)
•November 16, 2009 • Leave a CommentThis bug was recently disclosed by Sun Microsystems and it affects 1.6, 2.0.0, 2.0.2, 2.0.4, 2.0.6, 2.0.8, 2.0.10, 2.1, 2.2, 3.0.0, 3.0.2, 3.0.4, 3.0.6 and 3.0.8 releases of “Guest Additions” software provided by Sun xVM VirtualBox.
Guest Additions is a software available for Windows, Linux and Solaris and it is used in the installed guest operating systems for performance and integration purposes.
So, back to the actual code. As we can read in src/VBox/Additions/common/VBoxGuest/VBoxGuest.cpp of 3.0.8 release of VirtualBox we can find the following…
static int VBoxGuestCommonIOCtl_VMMRequest(PVBOXGUESTDEVEXT pDevExt, PVBOXGUESTSESSION pSession,
VMMDevRequestHeader *pReqHdr, size_t cbData, size_t *pcbDataReturned)
{
Log(("VBoxGuestCommonIOCtl: VMMREQUEST type %d\n", pReqHdr->requestType));
/*
* Validate the header and request size.
*/
const VMMDevRequestType enmType = pReqHdr->requestType;
const uint32_t cbReq = pReqHdr->size;
const uint32_t cbMinSize = vmmdevGetRequestSize(enmType);
if (cbReq < cbMinSize)
...
if (cbReq > cbData)
...
/*
* Make a copy of the request in the physical memory heap so
* the VBoxGuestLibrary can more easily deal with the request.
* (This is really a waste of time since the OS or the OS specific
* code has already buffered or locked the input/output buffer, but
* it does makes things a bit simpler wrt to phys address.)
*/
VMMDevRequestHeader *pReqCopy;
int rc = VbglGRAlloc(&pReqCopy, cbReq, enmType);
if (RT_FAILURE(rc))
{
Log(("VBoxGuestCommonIOCtl: VMMREQUEST: failed to allocate %u (%#x) bytes to cache the request. rc=%d!!\n",
cbReq, cbReq, rc));
return rc;
}
memcpy(pReqCopy, pReqHdr, cbReq);
...
VbglGRFree(pReqCopy);
return rc;
}
The aboce IOCTL handler is used by the aforementioned guest addition utilities software to perform a request on the hypervisor VMM. The request’s information are stored in a ‘VMMDevRequestHeader’ object which is passed as a pointer using ‘pReqHdr’ variable.
After initializing some variables with the request type and size, a check takes place to ensure that the minimum default size for that request is larger than the size contained in the request header. After that, a second size check takes place to ensure that the requested data fit in the requested size. Assuming that both conditions are true, VirtualBox will attempt to copy the request in heap memory using VbglGRAlloc() to allocate the required space and memcpy() to copy the actual data of the request. However, the request might still request more memory than it actually needs since it only checks that the space is more than the minimum. Because of this missing check, an attacker can create a malicious call to that IOCTL with a header size large enough to consume a lot of kernel memory and bring the guest OS in an unstable state.
To fix this, the above function was changed like this:
return VERR_INVALID_PARAMETER;
}
+ int rc = VbglGRVerify(pReqHdr, cbData);
+ if (RT_FAILURE(rc))
+ {
+ Log(("VBoxGuestCommonIOCtl: VMMREQUEST: invalid header: size %#x, expected >= %#x (hdr); type=%#x; rc %d!!\n",
+ cbData, cbReq, enmType, rc));
+ return rc;
+ }
/*
And this VbglGRVerify() is a newly added routine which resides in src/VBox/Additions/common/VBoxGuestLib/GenericRequest.cpp like this:
DECLVBGL(int) VbglGRVerify (const VMMDevRequestHeader *pReq, size_t cbReq)
{
if (!pReq || cbReq < sizeof (VMMDevRequestHeader))
{
dprintf(("VbglGRVerify: Invalid parameter: pReq = %p, cbReq = %d\n", pReq, cbReq));
return VERR_INVALID_PARAMETER;
}
if (pReq->size > cbReq)
{
dprintf(("VbglGRVerify: request size %d > buffer size %d\n", pReq->size, cbReq));
return VERR_INVALID_PARAMETER;
}
/* The request size must correspond to the request type. */
size_t cbReqExpected = vmmdevGetRequestSize(pReq->requestType);
if (cbReq < cbReqExpected)
{
dprintf(("VbglGRVerify: buffer size %d < expected size %d\n", cbReq, cbReqExpected));
return VERR_INVALID_PARAMETER;
}
if (cbReqExpected == cbReq)
{
/* This is most likely a fixed size request, and in this case the request size
* must be also equal to the expected size.
*/
if (pReq->size != cbReqExpected)
{
dprintf(("VbglGRVerify: request size %d != expected size %d\n", pReq->size, cbReqExpected));
return VERR_INVALID_PARAMETER;
}
return VINF_SUCCESS;
}
/* This can be a variable size request. Check the request type and limit the size
* to VMMDEV_MAX_VMMDEVREQ_SIZE, which is max size supported by the host.
*/
if ( pReq->requestType == VMMDevReq_LogString
|| pReq->requestType == VMMDevReq_VideoSetVisibleRegion
|| pReq->requestType == VMMDevReq_SetPointerShape
#ifdef VBOX_WITH_64_BITS_GUESTS
|| pReq->requestType == VMMDevReq_HGCMCall32
|| pReq->requestType == VMMDevReq_HGCMCall64
#else
|| pReq->requestType == VMMDevReq_HGCMCall
#endif /* VBOX_WITH_64_BITS_GUESTS */
|| pReq->requestType == VMMDevReq_ChangeMemBalloon)
{
if (cbReq > VMMDEV_MAX_VMMDEVREQ_SIZE)
{
dprintf(("VbglGRVerify: VMMDevReq_LogString: buffer size %d too big\n", cbReq));
return VERR_BUFFER_OVERFLOW; /* @todo is this error code ok? */
}
}
else
{
dprintf(("VbglGRVerify: request size %d > buffer size %d\n", pReq->size, cbReq));
return VERR_IO_BAD_LENGTH; /* @todo is this error code ok? */
}
return VINF_SUCCESS;
}
What it does is a series of checks the validate the request header’s contents. Specifically, it checks the following:
- The request is not NULL
- Data have at least the size of a valid request object
- The request’s size is larger than the data to be copied
- The data’s size cannot be less than the request’s expected size
- If the data’s size is equal to the expected one and the request’s size is not different of the expected one, return with success
- if the request type is VMMDevReq_LogString, VMMDevReq_VideoSetVisibleRegion, VMMDevReq_SetPointerShape as well as some 64-bit OS specific ones which are: VMMDevReq_HGCMCall32, VMMDevReq_HGCMCall64 and VMMDevReq_HGCMCall, check that the data aren’t larger than VMMDEV_MAX_VMMDEVREQ_SIZE constant.
Using this, the IOCTL handler will catch any invalid request that could lead in memory consumption and eventually a DoS situation in the guest OS when Guest Additions software is installed.
Similar checks were also added in VBoxGuestDeviceControl() function located in src/VBox/Additions/WINNT/VBoxGuest/VBoxGuest.cpp like this:
/* make sure the buffers suit the request */
CHECK_SIZE(vmmdevGetRequestSize(requestHeader->requestType));
+ int rc = VbglGRVerify(requestHeader, requestHeader->size);
+ if (RT_FAILURE(rc))
+ {
+ dprintf(("VBoxGuest::VBoxGuestDeviceControl: VMMREQUEST: invalid header: size %#x, expected >= %#x (hdr); type=%#x; rc %d!!\n",
+ requestHeader->size, vmmdevGetRequestSize(requestHeader->requestType), requestHeader->requestType, rc));
+ Status = STATUS_INVALID_PARAMETER;
+ break;
+ }
+
/* just perform the request */
VMMDevRequestHeader *req = NULL;
- int rc = VbglGRAlloc((VMMDevRequestHeader **)&req, requestHeader->size, requestHeader->requestType);
+ rc = VbglGRAlloc((VMMDevRequestHeader **)&req, requestHeader->size, requestHeader->requestType);
if (RT_SUCCESS(rc))
As well as vboxadd_ioctl() located in src/VBox/Additions/linux/module/vboxmod.c…
if (0 == rc)
{
+ rc = VbglGRVerify(&reqHeader, cbRequestSize);
+ if (RT_FAILURE(rc))
+ {
+ Log(("VBOXGUEST_IOCTL_VMMREQUEST: invalid request header: size %d min: %d type: %d rc: %d\n",
+ cbRequestSize,
+ cbVanillaRequestSize,
+ reqHeader.requestType,
+ rc));
+ rc = -EINVAL;
+ }
+ else
+ rc = 0;
+ }
+ if (0 == rc)
+ {
/* request storage for the full request */
And at last, the official API definition for the newly added function can be found in include/VBox/VBoxGuestLib.h.
/** * Verify the generic request header. * * @param pReq pointer the request header structure. * @param cbReq size of the request memory block. It should be equal to the request size * for fixed size requests. It can be greater than the request size for * variable size requests. * * @return VBox status code. */ DECLVBGL(int) VbglGRVerify (const VMMDevRequestHeader *pReq, size_t cbReq);
CVE-2009-3379: Mozilla Firefox Multiple Vulnerabilities
•November 13, 2009 • 1 CommentThis CVE name which was released through MFSA-2009-63 is about a series of vulnerabilities mainly in the vorbis library code. I’m going to give a brief description of each one included so let’s start…
1) Vorbis Decoding Missing Checks
This one was reported by Lucas Adamski of Mozilla and the susceptible code resides in media/libfishsound/src/libfishsound/fishsound_vorbis.c, and specifically in the following routine.
#if FS_DECODE
static long
fs_vorbis_decode (FishSound * fsound, unsigned char * buf, long bytes)
{
FishSoundVorbisInfo * fsv = (FishSoundVorbisInfo *)fsound->codec_data;
ogg_packet op;
long samples;
float * pcm_new;
int ret;
/* Make an ogg_packet structure to pass the data to libvorbis */
...
if (fsv->packetno < 3) {
if ((ret = vorbis_synthesis_headerin (&fsv->vi, &fsv->vc, &op)) == 0) {
if (fsv->vi.rate != 0) {
debug_printf (1, "Got vorbis info: version %d\tchannels %d\trate %ld",
fsv->vi.version, fsv->vi.channels, fsv->vi.rate);
fsound->info.samplerate = fsv->vi.rate;
fsound->info.channels = fsv->vi.channels;
}
}
/* Decode comments from packet 1. Vorbis has 7 bytes of marker at the
* start of vorbiscomment packet. */
if (fsv->packetno == 1 && bytes > 7 && buf[0] == 0x03 &&
!strncmp ((char *)&buf[1], "vorbis", 6)) {
...
} else if (fsv->packetno == 2) {
vorbis_synthesis_init (&fsv->vd, &fsv->vi);
vorbis_block_init (&fsv->vd, &fsv->vb);
}
...
return 0;
}
#else /* !FS_DECODE */
#define fs_vorbis_decode NULL
#endif
This function is used to decode an OGG file structure. In case of a packet number less than three, it will invoke vorbis_synthesis_headerin() to parse the vorbis information (represented through ‘&fsv->vi’) and the vorbis comments’ section (stored in ‘&fsv->vc’). The problem is that vorbis_synthesis_headerin() can fail in numerous situations as you can see below, the above code doesn’t check if it succeeds or fails. This can result in processing of an invalid vorbis structure.
/* The Vorbis header is in three packets; the initial small packet in
the first page that identifies basic parameters, a second packet
with bitstream comments and a third packet that holds the
codebook. */
int vorbis_synthesis_headerin(vorbis_info *vi,vorbis_comment *vc,ogg_packet *op){
oggpack_buffer opb;
...
if(memcmp(buffer,"vorbis",6)){
/* not a vorbis header */
return(OV_ENOTVORBIS);
}
switch(packtype){
case 0x01: /* least significant *bit* is read first */
if(!op->b_o_s){
/* Not the initial packet */
return(OV_EBADHEADER);
}
if(vi->rate!=0){
/* previously initialized info header */
return(OV_EBADHEADER);
}
...
case 0x03: /* least significant *bit* is read first */
if(vi->rate==0){
/* um... we didn't get the initial header */
return(OV_EBADHEADER);
}
...
case 0x05: /* least significant *bit* is read first */
if(vi->rate==0 || vc->vendor==NULL){
/* um... we didn;t get the initial header or comments yet */
return(OV_EBADHEADER);
}
...
default:
/* Not a valid vorbis header type */
return(OV_EBADHEADER);
break;
}
}
}
return(OV_EBADHEADER);
}
This was patched by adding the missing checks like this:
fsound->info.channels = fsv->vi.channels;
}
+ } else if (ret < 0) {
+ /* error occured while decoding the header */
+ return -1;
}
Since all of the error codes returned by vorbis_synthesis_headerin() are negative numbers, this check is sufficient and it will immediately return with no further processing.
A similar vulnerability was also present in fs_vorbis_decode() when it was handling the second packet of a three-or-less OGG structure. As you can read in the above snippet, it calls vorbis_synthesis_init() and vorbis_block_init() to initialize some members of the vorbis structure, however, those routines could fail and leave the structure in an inconsistent state for further processing. This was fixed by applying the following patch:
} else if (fsv->packetno == 2) {
- vorbis_synthesis_init (&fsv->vd, &fsv->vi);
- vorbis_block_init (&fsv->vd, &fsv->vb);
+ if (vorbis_synthesis_init (&fsv->vd, &fsv->vi) != 0)
+ return -1;
+
+ if (vorbis_block_init (&fsv->vd, &fsv->vb) != 0)
+ return -1;
}
} else {
In addition, Lucas Adamski provided a PoC trigger OGG file that you download here.
2) Vorbis Codebook Integer Overflow
This next bug was reported to Mozilla by Reed Loden. However, for the original bug discovery (under CVE-2008-1423 name) Dan Kaminsky should be credited since he was the one who discover it.
The suscpetible code is available at the media/libvorbis/lib/vorbis_codebook.c which is the location of the libvorbis library. Here is a snippet of the vorbis_staticbook_unpack() routine…
/* unpacks a codebook from the packet buffer into the codebook struct,
readies the codebook auxiliary structures for decode *************/
int vorbis_staticbook_unpack(oggpack_buffer *opb,static_codebook *s){
long i,j;
memset(s,0,sizeof(*s));
s->allocedp=1;
...
/* first the basic parameters */
s->dim=oggpack_read(opb,16);
s->entries=oggpack_read(opb,24);
...
int quantvals=0;
switch(s->maptype){
case 1:
quantvals=(s->dim==0?0:_book_maptype1_quantvals(s));
break;
case 2:
quantvals=s->entries*s->dim;
break;
}
/* quantized values */
s->quantlist=_ogg_malloc(sizeof(*s->quantlist)*quantvals);
for(i=0;i<quantvals;i++)
s->quantlist[i]=oggpack_read(opb,s->q_quant);
...
return(-1);
}
As you can read from the function’s comments, this is used to unpack a codebook structure passed to it. The vulnerable code starts during the initialization of ’s->dim’ (which represents the codebook dimensions) and ’s->entries’ (which contains the codebook entries) using oggpack_read() function. This means that ’s->dim’ can have values up to 2^16-1 (that results in 65535) and ’s->entries’ up to 2^24-1 (which is 16777215). Next, it has a switch statement to check if there is a mapping to unpack, in case of ’s->maptype’ set to two (aka. listed arbitrary values) it will perform a calculation that could result in an integer overflow, that is:
s->entries * s->dim
If someone uses the maximum available values, this will result in the following:
65535 * 16777215 = 1099494785025
which will definitely not fit in the 32-bit long, singed integer ‘quantvals’. The subsequent allocation using _ogg_malloc() will lead to allocation of insufficient space and the following operations on that area will almost certainly result in heap memory corruption.
To fix this bug, the following patch was used:
if(s->entries==-1)goto _eofout;
+ if(_ilog(s->dim)+_ilog(s->entries)>24)goto _eofout;
+
/* codeword ordering.... length ordered or unordered? */
switch((int)oggpack_read(opb,1)){
This is a simple check that the sum of the bits used in ’s->dim’ and ’s->entries’ aren’t more than 24, since _ilog() simply returns the number of bits as you can read at media/libvorbis/lib/vorbis_sharedbook.c…
/**** pack/unpack helpers ******************************************/
int _ilog(unsigned int v){
int ret=0;
while(v){
ret++;
v>>=1;
}
return(ret);
}
3) libvorbis OGG Heap Buffer Overflows
This next vulnerability which was reported by Matthew Gregan can be found in media/libvorbis/lib/vorbis_res0.c in the function listed below.
/* vorbis_info is for range checking */
vorbis_info_residue *res0_unpack(vorbis_info *vi,oggpack_buffer *opb){
int j,acc=0;
vorbis_info_residue0 *info=_ogg_calloc(1,sizeof(*info));
codec_setup_info *ci=vi->codec_setup;
...
info->partitions=oggpack_read(opb,6)+1;
info->groupbook=oggpack_read(opb,8);
for(j=0;j<info->partitions;j++){
int cascade=oggpack_read(opb,3);
if(oggpack_read(opb,1))
cascade|=(oggpack_read(opb,5)<<3);
info->secondstages[j]=cascade;
acc+=icount(cascade);
}
for(j=0;j<acc;j++)
info->booklist[j]=oggpack_read(opb,8);
...
if(info->groupbook>=ci->books)goto errout;
for(j=0;j<acc;j++){
if(info->booklist[j]>=ci->books)goto errout;
if(ci->book_param[info->booklist[j]]->maptype==0)goto errout;
}
...
int entries = ci->book_param[info->groupbook]->entries;
int dim = ci->book_param[info->groupbook]->dim;
int partvals = 1;
while(dim>0){
partvals *= info->partitions;
if(partvals > entries) goto errout;
dim--;
}
info->partvals = partvals;
}
...
errout:
res0_free_info(info);
return(NULL);
}
The problem with the above code is that the heap allocated ‘info->groupbook’ which should contain the huffbook for partitioning is initialized using oggpack_read() from the user controlled OGG file. The same applies for ‘info->partitions’ too (this represents the possible codebooks for a partition). The next ‘for’ loop will iterate for every possible codebook and and update ‘cascade’ and ‘info->secondstages’ accordingly. However, since the processed value is also retrieved directly from the user controlled buffer, it could be negative which will later result in memory corruption.
This first vulnerability was patched using the following approach:
info->groupbook=oggpack_read(opb,8);
+ /* check for premature EOP */
+ if(info->groupbook<0)goto errout;
+
for(j=0;j<info->partitions;j++){
int cascade=oggpack_read(opb,3);
- if(oggpack_read(opb,1))
- cascade|=(oggpack_read(opb,5)<<3);
+ int cflag=oggpack_read(opb,1);
+ if(cflag<0) goto errout;
+ if(cflag){
+ int c=oggpack_read(opb,5);
+ if(c<0) goto errout;
+ cascade|=(c<<3);
+ }
info->secondstages[j]=cascade;
First of all, before even beginning the processing, they check for negative values in ‘info->groupbook’ and the first ‘for’ loop was changed to check that the retrieved value (which is now temporarily stored in ‘cflag’) is neither negative nor zero and the value to be OR’d with ‘cascade’ is also checked for not being negative.
Back to res0_unpack(), the second ‘for’ loop used to initialize ‘info->booklist[]‘ array directly from the OGG buffer was also patched in a similar manner…
acc+=icount(cascade);
}
- for(j=0;j<acc;j++)
- info->booklist[j]=oggpack_read(opb,8);
+ for(j=0;j<acc;j++){
+ int book=oggpack_read(opb,8);
+ if(book<0) goto errout;
+ info->booklist[j]=book;
+ }
if(info->groupbook>=ci->books)goto errout;
Again, same negative values’ checks before proceeding with the update.
At last, during the last code of the above snippet of res0_unpack() you can read that integers ‘entries’ and ‘dim’ are initialized with the codebook entries and the codebook dimensions respectively. Then, a ‘while’ loop takes place. In it there is a multiplication that could result in an integer overflow since ‘partvals’ is just a 32-bit long signed integer. To avoid overflows the following patch was used.
if(partvals > entries) goto errout;
dim--;
}
+ if(partvals < entries) goto errout;
}
So, before updating the ‘info->partvals’ (which should normally contain the “artitions ^ groupbook dimensions” as we can read at media/libvorbis/lib/backends.h) with the calculated ‘portvals’, it checks that the latter integer hasn’t overflowed to a negative value.
Matthew Gregan also gave a PoC OGG file to trigger the bug at will which you can find here.
4) libvorbis Missing Checks
This is the last bug under this CVE name which was reported by David Keeler of Stanford University. Here is the buggy code of vorbis_staticbook_unpack() as seen at media/libvorbis/lib/vorbis_codebook.c…
/* unpacks a codebook from the packet buffer into the codebook struct,
readies the codebook auxiliary structures for decode *************/
int vorbis_staticbook_unpack(oggpack_buffer *opb,static_codebook *s){
long i,j;
memset(s,0,sizeof(*s));
s->allocedp=1;
...
/* codeword ordering.... length ordered or unordered? */
switch((int)oggpack_read(opb,1)){
case 0:
/* unordered */
...
case 1:
/* ordered */
{
long length=oggpack_read(opb,5)+1;
s->lengthlist=_ogg_malloc(sizeof(*s->lengthlist)*s->entries);
for(i=0;i<s->entries;){
long num=oggpack_read(opb,_ilog(s->entries-i));
if(num==-1)goto _eofout;
for(j=0;j<num && i<s->entries;j++,i++)
s->lengthlist[i]=length;
length++;
}
}
break;
default:
/* EOF */
return(-1);
}
...
_errout:
_eofout:
vorbis_staticbook_clear(s);
return(-1);
}
The bug with this part of the routine is the case of an ordered codeword length. The long signed integer ‘length’ is initialized using user controlled OGG buffer and ’s->lengthlist’ points to some heap allocated space that was obtained using _ogg_malloc() as you can read in the given code. The next part is a simple ‘for’ loop that iterates for each codebook entry and updates all of their ’s->lengthlist[]‘ array entries with the previously retrieved ‘length’. However, this value cannot be greater than 32 since it is used to contain the codeword length in bits!
Nevertheless, there is no check to ensure this, and an attacker could have complete control over this value. To fix this, the following patch was applied:
long num=oggpack_read(opb,_ilog(s->entries-i));
if(num==-1)goto _eofout;
+ if(length>32)goto _errout;
for(j=0;j<num && i<s->entries;j++,i++)
Quite expected patch.
Additionally, David Keeler uploaded a PoC OGG file that can be used to trigger the vulnerability. This file can be downloaded here.
That’s all… I believe that those are a many bugs to be included under a single CVE name but they might know better. In any case, have fun!
CVE-2009-3378: Mozilla Firefox liboggplay Uninitialized Pointer Dereference
•November 13, 2009 • Leave a CommentThe last vulnerability being released in MFSA-2009-63 was this one. This bug was reported by Juan Becerra of Mozilla and it can be found in media/liboggplay/src/liboggplay/oggplay_data.c file which is part of the liboggplay library.
int
oggplay_data_handle_theora_frame (OggPlayTheoraDecode *decode,
const yuv_buffer *buffer) {
size_t size = sizeof (OggPlayVideoRecord);
int i, ret;
long y_size, uv_size, y_offset, uv_offset;
unsigned char * p;
unsigned char * q;
unsigned char * p2;
unsigned char * q2;
OggPlayVideoRecord * record;
OggPlayVideoData * data;
...
/*
* *grumble* theora plays silly buggers with pointers so we need to do
* a row-by-row copy (stride may be negative)
*/
y_offset = (decode->video_info.offset_x&~1)
+ buffer->y_stride*(decode->video_info.offset_y&~1);
p = data->y;
q = buffer->y + y_offset;
for (i = 0; i < decode->y_height; i++) {
memcpy(p, q, decode->y_width);
p += decode->y_width;
q += buffer->y_stride;
}
...
return E_OGGPLAY_CONTINUE;
}
Pointer ‘buffer’ is used to contain uncompressed frames to and from the codec. The given snippet is a simple copy operation of the decoded width. However, there is a case where ‘buffer->y’ would be uninitialized and the memcpy() will attempt to copy data from uninitialized memory and thus, result in a uninitialized pointer dereference.
The exact code path to achieve this is the following…
int theora_decode_packetin(theora_state *_td,ogg_packet *_op){
th_api_wrapper *api;
ogg_int64_t gp;
int ret;
if(!_td||!_td->i||!_td->i->codec_setup)return OC_FAULT;
api=(th_api_wrapper *)_td->i->codec_setup;
ret=th_decode_packetin(api->decode,_op,&gp);
if(ret<0)return OC_BADPACKET;
_td->granulepos=gp;
return 0;
}
This will be called first but th_decode_packetin() can return TH_DUPFRAME (meaning that it doesn’t exist in the Theora stream), as we can read at media/libtheora/include/theora/codec.h this has a positive value:
#define TH_DUPFRAME (1) /*@}*/
Consequently, theora_decode_packetin() will return zero and the next function that will be called is the theora_decode_YUVout() located in media/libtheora/lib/decapiwrapper.c too.
int theora_decode_YUVout(theora_state *_td,yuv_buffer *_yuv){
th_api_wrapper *api;
th_dec_ctx *decode;
th_ycbcr_buffer buf;
int ret;
if(!_td||!_td->i||!_td->i->codec_setup)return OC_FAULT;
api=(th_api_wrapper *)_td->i->codec_setup;
decode=(th_dec_ctx *)api->decode;
if(!decode)return OC_FAULT;
ret=th_decode_ycbcr_out(decode,buf);
if(ret>=0){
_yuv->y_width=buf[0].width;
_yuv->y_height=buf[0].height;
_yuv->y_stride=buf[0].stride;
_yuv->uv_width=buf[1].width;
_yuv->uv_height=buf[1].height;
_yuv->uv_stride=buf[1].stride;
_yuv->y=buf[0].data;
_yuv->u=buf[1].data;
_yuv->v=buf[2].data;
}
return ret;
}
which doesn’t perform any operations on the “_yuv” buffer and thus, it will return with no error too. This means that even though the ‘yuv_buffer’ is still uninitialized, oggplay_data_handle_theora_frame() will be invoked and attempt to copy the width and dereference an uninitialized pointer.
To fix this, Mozilla developers changed the oggplay_initialise() initialization routine found in media/liboggplay/src/liboggplay/oggplay.c to set the decoder to inactive which was missing. Here is the patch:
/*
* set all the tracks to inactive
*/
for (i = 0; i < me->num_tracks; i++) {
me->decode_data[i]->active = 0;
}
+ me->active_tracks = 0;
As you can read, even though the ‘decode_data[]‘ array was cleared, the active tracks for decoding was not. This patch updates it to be initialized to zero too. And of course, the equivalent Theora routine which resides in media/liboggplay/src/liboggplay/oggplay_callback.c under oggplay_init_theora() was changed to increment the OGG player handle’s active tracks like this:
decoder->frame_delta = 0; decoder->y_width = 0; decoder->convert_to_rgb = 0; decoder->decoder.decoded_type = OGGPLAY_YUV_VIDEO; + decoder->decoder.player->active_tracks++; }
Finally, oggplay_initialise_decoder() was changed in order to set the decoder to active.
decoder->content_type_name =
oggz_stream_get_content_type (me->oggz, serialno);
- decoder->active = 0;
+ decoder->active = 1;
decoder->final_granulepos = -1;
decoder->player = me;
As Matthew Gregan pointed out, by enabling the decoder, it will decode the packets before they reach the code that causes the crash. This is why this approach was used to fix the vulnerability.
Finally, M. Gregan also gave a PoC OGG file that triggers the bug and can be downloaded here.
FreeBSD fmtmsg(3) Typo
•November 12, 2009 • 1 CommentThis is a funny bug reported by soulcatcher to the FreeBSD project. The vulnerable code resides in gen/fmtmsg.c and specifically in printfmt() routine as you can see below.
/*
* Returns NULL on memory allocation failure, otherwise returns a pointer to
* a newly malloc()'d output buffer.
*/
static char *
printfmt(char *msgverb, long class, const char *label, int sev,
const char *text, const char *act, const char *tag)
{
size_t size;
char *comp, *output;
const char *sevname;
size = 32;
...
if (text != MM_NULLTXT)
size += strlen(text);
if (text != MM_NULLACT)
size += strlen(act);
...
if ((output = malloc(size)) == NULL)
...
return (output);
}
This could be reached directly using fmtmsg(3).
The problem here is in the ‘MM_NULLACT’ case (which checks if ‘act’ is set to “NULL” or not), which is incorrectly using ‘text’ argument even though it updates the size variable using ‘act’. The advisory included a small PoC trigger C code which you can see here.
#include <fmtmsg.h>
int main(int argc, char * argv[])
{
fmtmsg(MM_UTIL | MM_PRINT, "BSD:ls", MM_ERROR,
"illegal option -- z", MM_NULLACT, "BSD:ls:001");
return 0;
}
It is quite simple, it has a ‘text’ argument set to “illegal option — z” and ‘act’ set to MM_NULLACT. This will attempt to add the length of that ‘MM_NULLACT’ to the ’size’ and eventually, result in a segmentation fault.
The fix was, as you might have guessed…
if (text != MM_NULLTXT)
size += strlen(text);
- if (text != MM_NULLACT)
+ if (act != MM_NULLACT)
size += strlen(act);
It just checks the correct argument before updating the size that will be later allocated.
