xorl %eax, %eax

Linux kernel IrDA Nickname Heap Corruption

leave a comment »

Another bug reported by Dan Rosenberg this time affecting Linux kernel before release. The susceptible code is available at net/irda/irnet/irnet_ppp.c where we can find the IrNET protocol module for synchronous PPP over an IrDA socket.

 * Write is used to send a command to configure a IrNET channel
 * before it is open by pppd. The syntax is : "command argument"
 * Currently there is only two defined commands :
 *      o name : set the requested IrDA nickname of the IrNET peer.
 *      o addr : set the requested IrDA address of the IrNET peer.
 * Note : the code is crude, but effective...
static inline ssize_t
irnet_ctrl_write(irnet_socket * ap,
                 const char __user *buf,
                 size_t         count)
  char          command[IRNET_MAX_COMMAND];
  char *        start;          /* Current command being processed */
  char *        next;           /* Next command to process */
  int           length;         /* Length of current command */
  /* Check for overflow... */
         CTRL_ERROR, "Too much data !!!\n");

  /* Get the data in the driver */
  if(copy_from_user(command, buf, count))
      /* First command : name -> Requested IrDA nickname */
      if(!strncmp(start, "name", 4))
          /* Copy the name only if is included and not "any" */
          if((length > 5) && (strcmp(start + 5, "any")))
              /* Strip out trailing whitespaces */
              while(isspace(start[length - 1]))

              /* Copy the name for later reuse */
              memcpy(ap->rname, start + 5, length - 5);
              ap->rname[length - 5] = '\0';

  /* Success : we have parsed all commands successfully */
  return count;

This code parses a user derived command but it lacks of some important checks. The constant ‘IRNET_MAX_COMMAND’ is defined in net/irda/irnet/irnet_ppp.h header file like this:

/* IrNET control channel stuff */
#define IRNET_MAX_COMMAND       256     /* Max length of a command line */

So, if the user command begins with the string “name” and it isn’t followed by “any” string value, it will reach the memcpy() copy operation. As Dan Rosenberg pointed out, using names containing only white-spaces, this will end up underflowing integer ‘length’ in the ‘while’ loop of the given code snippet that decrements ‘length’ for each trailing white space it encounters. Because of this, the subsequent subtraction in the memcpy() call will result in kernel heap memory corruption for length values of less than 5.
The fix to this bug was to add some additional checks after that loop.

 	      while(isspace(start[length - 1]))
+	      DABORT(length < 5 || length > NICKNAME_MAX_LEN + 5,
+		     -EINVAL, CTRL_ERROR, "Invalid nickname.\n");
 	      /* Copy the name for later reuse */
 	      memcpy(ap->rname, start + 5, length - 5);

It checks that the length is not less than 5 to avoid integer underflows as well as that it does not exceed the nickname limit which is defined in include/net/irda/discovery.h header file.


The second bound checking is crucial since large nickname values result in controlled heap corruption of ‘ap->rname’ that is defined in net/irda/irnet/irnet.h header file with static size as shown below.

 * This is the main structure where we store all the data pertaining to
 * one instance of irnet.
 * Note : in irnet functions, a pointer this structure is usually called
 * "ap" or "self". If the code is borrowed from the IrDA stack, it tend
 * to be called "self", and if it is borrowed from the PPP driver it is
 * "ap". Apart from that, it's exactly the same structure ;-)
typedef struct irnet_socket
  char                  rname[NICKNAME_MAX_LEN + 1];
} irnet_socket

Written by xorl

April 23, 2011 at 18:21

Posted in bugs, linux

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s