[bugzilla-daemon@xxxxxxxxxxxxx]

Comment # 9 on bug 8635 from Peter Van Eynde

Hello Jörg,

(In reply to comment #8)
> So looking into the current version with some more time:
> - I already asked why you changed some variables from FT_IPv4 to FT_UINT32.

Sorry I must have missed that. Where did you ask this?

> While I now understand that these values are printed via
> wccp_fmt_ipadddress, this is not equivalent as we loose the special features
> that we get by using FT_IPv4 and FT_IPv6. If would be good to add "normal"
> addresses via the correct FT_IPvX types and only print strings when there is
> a special case (error).

I understand and prefer that way in fact. However IPv6 support in WCCP is done
by adding a new section which translates UNIT32 indices into IP addresses.

See http://tools.ietf.org/html/draft-mclaggan-wccp-v2rev1-00#page-38

So the binary value is either FT_IPv4 if there is no address table component,
or it can be index which we can translate to a IPv4 or IPv6 address using the
address table component.

I have found no clean way to implement this besides the custom formatter, what
better way is there to implement this?

> - In find_wccp_address_table: I'm not sure (due to lack of knowledge of the
> c standard) that there can't be an integer overflow in the while loop.

I can see your concern, at the moment I so:

   ..
  guint16 item_length, length;

   while (length >= 4) {
    item_length = ...

    /* check if we _can_ advance */
    if (length < (item_length+4))
      return;

    offset = offset + item_length + 4;
    length = length - item_length - 4;
  }

your concern is that if for example length is 4 and item_length is 0xFFFFFFFE
that (item_length + 4) is 3. 4 > 3 so length becomes 4 - 0xFFFFFFFE - 4  -> 2. 

As far as I can see the loop should be overflow free.

> Anyway doing an extra check of the item_length when read from the packet
> would make sense.

Checking if item_length <= tvb_length_remaining(tvb,offset) ?

> - NOTE_EATEN_LENGTH is probably a tpyo

Sorry can't see this, what should it be?

Thanks for the review! Peter

---

      You are receiving this mail because:

• You are watching all bug changes.