vsftpd-2.2.2 released

Just a quick note that I released vsftpd-2.2.2.
Most significantly, a regression was fixed in the inbuilt listener. Heavily loaded sites could see a session get booted out just after the initial connect. If you saw "500 OOPS: child died", that was probably this.

http://vsftpd.beasts.org/

A new fuzz frontier: packet boundaries

Recently, I've been getting pretty behind on executing my various research ideas. The only sane thing to do is blog the idea in case someone else wants to run with it and pwn up a bunch of stuff.

The general concept I'd like to see explored is perhaps best explained with a couple of concrete bugs I have found and fixed recently:

1. Dimensions error parsing XBM image. Welcome to the XBM image format, a veritable dinosaur of image formats. It's a textual format and looks a bit like this:
   

   #define test_width 8
   #define test_height 14
   static char test_bits[] = {
   0x13, 0x00, 0x15, 0x00, 0x93, 0xcd, 0x55, 0xa5, 0x93, 0xc5, 0x00, 0x80,
   0x00, 0x60 };
   

   The WebKit XBM parsing code includes this line, to extract the width and height:
   

           if (sscanf(&input[m_decodeOffset], "#define %*s %i #define %*s %i%n",
                      &width, &height, &count) != 2)
               return false;
   

   
   The XBM parser supports streaming (making render progress before you have the full data available), including streaming in the header. i.e. the above code will attempt to extract width and height from a partial XBM, and retry with more data if it fails. So what happens if the first network packet contains an XBM fragment of exactly the first 42 bytes of the above example? This looks like:
   

   #define test_width 8
   #define test_height 1
   

   I think you can see where this is going. The sscanf() sees two valid integers, and XBM decoding proceeds for an 8x1 image, which is incorrect. The real height, 14, had its ASCII representation fractured across a packet boundary.
   


2. Out-of-bounds read skipping over HTML comments. This is best expressed in terms of part of the patch I submitted to fix it:
   

   --- WebCore/loader/TextResourceDecoder.cpp (revision 44821)
   +++ WebCore/loader/TextResourceDecoder.cpp (working copy)
   @@ -509,11 +509,13 @@ bool TextResourceDecoder::checkForCSSCha
    static inline void skipComment(const char*& ptr, const char* pEnd)
    {
        const char* p = ptr;
   +    if (p == pEnd)
   +      return;
        // Allow <!-->; other browsers do.
        if (*p == '>') {
            p++;
        } else {
   -        while (p != pEnd) {
   +        while (p + 2 < pEnd) {
                if (*p == '-') {
                    // This is the real end of comment, "-->".
                    if (p[1] == '-' && p[2] == '>') {
   

   
   As can be seen, some simple bounds checking was missing. In order to trigger, the browser would need to find itself processing an HTML fragment ending in:
   

   <!--
   

   (Where "ending in" means not necessarily the end of the HTML, but the end of the HTML that we have received so far).
   

The general principle here? Software seems to have a lot of failure conditions with partial packets! This is unsurprising when you think about it; software is frequently trying to make progress based on partial information -- whether it's image or HTML parsers trying to show progress to the user, or network servers trying to extract a useful header or state transition from a short packet.
Typical fuzzing may not be able to trigger these conditions. I've certainly fuzzed image codecs using local files as inputs. This would never exercise partial packet code paths.
The best way to shake out these bugs is going to be to fuzz the packet boundaries at the same time as the packet data. Let me know if you find anything interesting :)

Chromium and Linux sandboxing

It was great to talk to so many people about Chromium security at HITB Malaysia. I was quite amused to be at a security conference and have a lot of conversations like:

Me: What browser do you use?
Other: Google Chrome.
Me: Why is that?
Other: Oh, it's so much faster.
Me: Oh, you saw that awesome JSNES, huh? (http://benfirshman.com/projects/jsnes/)

It's a sobering reminder that users -- and even security experts -- are often making decisions on things like speed and stability. It was similar with vsftpd. I set out to build the most secure FTP server, but usage took off unexpectedly because of the speed and scalability.

Julien talked about his clever Linux sandboxing trick that is used in the Chromium Linux port. One component of the sandbox is an empty chroot() jail, but setting up such a jail is a pain on many levels. The problems and solutions are as follows:

• chroot() needs root privilege. Therefore, a tiny setuid wrapper binary has been created to execute sandboxed renderer processes. Some people will incorrectly bleat and moan about any new setuid binary, but the irony is that is it required to make your browser more secure. Also, a setuid binary can be made small and careful. It will only execute a specific trusted binary (the Chromium renderer) inside an empty jail.


• exec()ing something from inside an empty jail is hard, because your view of the filesystem is empty. You could include copies of the needed dynamic libraries or a static executable but both of these are a maintenance and packaging nightmare. This is where Julien's clever tweak comes in. By using the clone() flag CLONE_FS, and sharing the FS structure between a trusted / privileged thread and the exec()ed renderer, the trusted thread can call chroot() and have it affect the unprivileged, untrusted renderer process post-exec. Neat, huh?


• Other tricks such as CLONE_NEWPID and CLONE_NEWNET are used or will be used to prevent sending of signals from a compromised renderer, and network access.

Finally, it is worth noting that sandboxing vs. risks on the web are widely misunderstood. The current Chromium sandbox mitigates Critical risk bugs to High risk bugs. This may be enhanced in the future. Since any bugs within the sandbox are still High risk, I of course take them very seriously and fix them as a priority. But, that lowering of a certain percentage of your bugs away from Critical risk is really key. The vast majority of web pwnage out there is enabled by Critical risk bugs (i.e. full compromise of user account => ability to install malware), so mitigating any of these down to High is a huge win. It's easy to get excited about any security bug, we as an industry really need to get more practical and concentrate on the ones actually causing damage.

Attacking this point from another angle: any complicated software will inevitably have bugs, and a certain subset of bugs are security bugs. Note that any web browser is certainly a complicated piece of software :) Therefore, any web browser is always going to be having security bugs. And indeed, IE, Opera, Firefox, Safari and Chrome are issuing regular security updates. For some reason, the media reports on each and every patch as if it is a surprising or relevant event. The real question, of course, is what you do in the face of the above realization. The Chromium story is two powerful mitigations: sandboxing to reduce severity away from Critical, and a very fast and agile update system to close any window of risk.

vsftpd-2.2.1 released

Nothing too exciting, just two regressions fixed: "pasv_address" should work again, and SSL data connections should no longer fail after a long previous transfer or an extended idle period.

HITB Malaysia 2009 and sandboxing

No time for details at the moment, but I'm just back from HITB Malaysia and a great time was had by all! The hospitality and warmth of the organizing crew surpassed anything I've ever encountered before.

I presented with my colleague Julien Tinnes. See awesome blog:

http://blog.cr0.org/

We presented on various intriguing aspects of sandboxing on Linux, covering vsftpd and Chromium as test cases. Our slides are located here:

Security in Depth for Linux Software

As per other presentations, I'll leave it at that for now and follow up with a mini series of posts for the more interesting points. I think vsftpd is well covered by previous posts, but Chromium on Linux is awesome and its built-in sandboxing deserves a few notes.

Patching ffmpeg into shape

Preface: unless otherwise noted, the bugs discussed here were found via fuzzing by Will Dormann of CERT -- and my involvement was to fix them. In other news, I recently moved to work on the Chromium project / Google Chrome, which I'm very excited about. It is in this new role that this piece of work was conducted, as part of HTML5 features.

I recently fixed a lot of security bugs in ffmpeg, across a subset of the supported containers and codecs. The bugs represented a range of different C vulnerability classes, which I thought might make an interesting blog post.

1. Out-of-bounds array index read in vorbis_dec.c:

   
       mapping_setup->submap_floor[j]=get_bits(gb, 8);
       mapping_setup->submap_residue[j]=get_bits(gb, 8);
   ...
       floor=&vc->floors[mapping->submap_floor[0]];
   ...
       no_residue[i]=floor->decode(vc, &floor->data, ch_floor_ptr);
   

   
   The index usage is far from where it is read, making this trickier to find by code auditing. Note how this is exploitable, despite being an out-of-bounds read. The out-of-bounds memory is used as a function pointer! It's easy to forget that out-of-bounds reads can be very serious.
   


2. Off-by-one indexing error in vp3.c:

   
               for (j = 0; j < run_length; i++) {
                   if (i > s->coded_fragment_list_index)
                       return -1;
   
                   if (s->all_fragments[s->coded_fragment_list[i]].qpi == qpi) {
   

   
   The > should be >= otherwise there is an out-of-bounds read. In this instance, the out-of-bounds read is for an index that is used to read and write to another array, so you have possible memory corruption as a consequence.
   Found by me with some additional fuzzing.
   


3. Interesting pointer arithmetic bug in oggparsevorbis.c:

   
       while (p < end && n > 0) {
           const char *t, *v;
           int tl, vl;
   
           s = bytestream_get_le32(&p);
   
           if (end - p < s)
               break;
   
           t = p;
           p += s;
   

   
   The subtlety here is that bytestream_get_le32() advances the pointer passed to it, by sizeof(int). If the current position pointer p points to, say, 3 bytes of remaining buffer, then we have two problems. Firstly, the integer read will be compromised of one byte of out-of-bounds data. More seriously, the condition p > end will end up being true. Subsequently, a very large value of s may escape the check against end - p. This will lead to attacker-controlled out-of-bounds reads later.
   


4. Assignment vs. comparison operator mix-up in vorbis_dec.c:

   
       for(i=0;i<mapping->submaps;++i) {
           vorbis_residue *residue;
           uint_fast8_t ch=0;
   
           for(j=0;j<vc->audio_channels;++j) {
               if ((mapping->submaps==1) || (i=mapping->mux[j])) {
   

   
   Do you see it? This is actually a serious bug because these loops are writing to an output heap buffer, and the loops have been carefully checked so that they will terminate before they overflow said buffer! Unfortunately, assigning to the loop iterator from an attacker-controlled variable will permit the attacker to conduct a heap overflow.
   Found by me via code auditing.
   


5. Integer underflow leading to stack pointer wrap-around in vorbis_dec.c:

   
       uint_fast16_t n_to_read=vr->end-vr->begin;
       uint_fast16_t ptns_to_read=n_to_read/vr->partition_size;
       uint_fast8_t classifs[ptns_to_read*vc->audio_channels];
   

   
   All sorts of interesting stuff going on here. For a start, missing validations parsing the header earlier do not prevent begin > end, leading to an integer underflow. Furthermore, uint_fast16_t on my platform at least is a 32-bit wide type. This enables the calculation for the size of the stack-based array to result in any 32-bit value too. On a 32-bit platform, this can result in the stack pointer wrapping around and moving "up" rather than "down". A usually exploitable condition. Even creating a huge (but non-wrap-causing) stack frame can have consequences depending on whether your system does a simple calculation on the stack pointer, or is more careful to fault each new page in case an end-of-stack guard page exists.
   


6. Integer underflow by 1 in mov.c:

   
   static int mov_read_elst(MOVContext *c, ByteIOContext *pb, MOVAtom atom)
   {
       MOVStreamContext *sc = c->fc->streams[c->fc->nb_streams-1]->priv_data;
   ...
               sc->time_offset = time != -1 ? time : -duration;
   

   
   The problem here is that it is assumed that nb_streams > 0 but no such condition is guaranteed if the attacker supplies an "elst" tag before supplying any tag that creates a stream. The result is that a pointer is used from an out-of-bounds location. Use of any invalid or corrupt pointer is usually an exploitable condition, of course.
   


7. Type confusion bug in mov.c / utils.c:
   (No code sample)
   The most interesting bug in my opinion. It triggers when a corrupt ordering of tags in the MOV container sets up some variables such "codec type" is e.g. VIDEO whereas "codec id" is e.g. MP3. Subsequently, some core code passes a pointer to a stack-allocated video structure to the the mp3 decoder. The mp3 decoder, assuming it has a pointer to the (larger) audio structure, happily causes a stack-based buffer overflow. Cool.
   Found by me with some additional fuzzing.
   


8. Other:
   This blog post got too long. Other bug classes included divide by zero, infinite looping, stack-based buffer overflow due to missing bounds check, classic integer overflow, and likely others. See the patches if you are interested: http://src.chromium.org/viewvc/chrome/trunk/deps/third_party/ffmpeg/patches/
   

Epilogue: The potential for bugs like these is why Chromium runs codecs inside its built-in sandbox. Although bugs inside the sandbox are still taken seriously, there are no longer of "Critical" severity. The real user damage on the web at this time is conducted by malware exploiting "Critical" severity bugs in some browser or plug-in.

vsftpd-2.2.0 released

Not much of interest to add beyond the interesting network isolation support previously discussed. Some minor bugs were fixed. A bunch of compile errors were addressed. There is now support for PAM modules which remap the underlying user account. There is also a new command-line option to pass config file options directly.