:+       data_len = ip_len - ((u_char *)cmn_hdr - (u_char *)ip);
:+
:...
:+       buf += sizeof(*cmn_hdr);
:+       data_len -= sizeof(*cmn_hdr);
:+
:+       while (data_len > 0) {
:+              obj_hdr = (struct icmp_ext_obj_hdr *)buf;
:+              obj_len = ntohs(obj_hdr->length);

    This should be data_len >= sizeof(struct icmp_ext_obj_hdr), 
    not >= 0.
        
         while (data_len >= sizeof(struct icmp_ext_obj_hdr)) {
                ...

:+
:+              /*
:+               * Sanity check the length field
:+               */
:+              if (obj_len > data_len) {
:+                      return;
:+              }

    obj_len can be 0.  Check that obj_len < sizeof(*obj_hdr)
    and return if it isn't.  obj_len can be an odd number, which
    is also not a good idea.  Note sure about alignment requirements,
    it might even have to be 4-byte aligned.  But 2-byte for sure.

                if (obj_len < sizeof(*obj_hdr) || obj_len > data_len)
                        return;
                if (obj_len & 3)        /* either & 1 or & 3, depending */
                        return;

:+
:+              data_len -= obj_len;
:+
:+              /*
:+               * Move past the object header
:+               */
:+              buf += sizeof(struct icmp_ext_obj_hdr);
:+              obj_len -= sizeof(struct icmp_ext_obj_hdr);
:+
:+              switch (obj_hdr->class_num) {
:+              case MPLS_STACK_ENTRY_CLASS:
:+                      switch (obj_hdr->c_type) {
:+                      case MPLS_STACK_ENTRY_C_TYPE:
:+                              while (obj_len >= (int)sizeof(uint32_t)) {
:+                                      mpls_hdr = ntohl(*(uint32_t *)buf);
:+
:+                                      buf += sizeof(uint32_t);
:+                                      obj_len -= sizeof(uint32_t);
:+                                      printf(" [MPLS: Label %d Exp %d]",
:+                                          MPLS_LABEL(mpls_hdr), 
MPLS_EXP(mpls_hdr));
:+                              }
:+                              if (obj_len > 0) {
:+                                      /*
:+                                       * Something went wrong, and we're at
:+                                       * a unknown offset into the packet,
:+                                       * ditch the rest of it.
:+                                       */
:+                                      return;
:+                              }
:+                              break;
:+                      default:
:+                              /*
:+                               * Unknown object, skip past it
:+                               */
:+                              buf += ntohs(obj_hdr->length) -
:+                                  sizeof(struct icmp_ext_obj_hdr);

    Hmm.  This is a bit messy

:...
:+                      /*
:+                       * Unknown object, skip past it
:+                       */
:+                      buf += ntohs(obj_hdr->length) -
:+                          sizeof(struct icmp_ext_obj_hdr);

    Same... a bit messy.

    I suggest declaring a 'nextbuf' which you calculate up at the
    'Move past the object header' code and just assign buf = nextbuf
    in these two places.

:+                      break;
:+              }
:+      }
: }

                                        -Matt
                                        Matthew Dillon 
                                        <[EMAIL PROTECTED]>

Reply via email to