Wireshark · Wireshark-bugs: [Wireshark-bugs] [Bug 12119] Buildbot crash output: fuzz-2016-02-11-28355.pcap

[bugzilla-daemon@xxxxxxxxxxxxx]

Comment # 6 on bug 12119 from Diederik de Groot

(In reply to Diederik de Groot from comment #5)
> (In reply to Michael Mann from comment #4)
> > I think the best solution would be to try to get the "tap data" (si
> > variable) as close to the top of a function as possible.  Then you could put
> > a if (ptvcursor_tree(cursor) == NULL) right after all of the uses. 
> > Hopefully that can eliminate all of the loops.
> > Barring that, you need to have sanity checks after grabbing any loop value
> > from the packet and checking if its loop size is too big for the packet.
> > Using this capture as an example, in handle_UpdateCapabilitiesV3Message,
> > there should be a check if (audioCapCount * "size of audio capability
> > structure") is greater than reported length of tvb, then return from
> > function.
> 
> Would adding this do the trick (3rd line)?
> 
>     guint32 counter_1 = 0;
>     ptvcursor_add_text_with_subtree(cursor, SUBTREE_UNDEFINED_LENGTH,
> ett_skinny_tree, "caps [ref: capCount = %d, max:18]", capCount);
>     if (hdr_data_length - ptvcursor_current_offset(cursor) >= (capCount *
> 16) && capCount <= 18) { /* tvb size guard */
>       for (counter_1 = 0; counter_1 < 18; counter_1++) {
>          ...
>       }
>     }
> 
> Where would you like me to add the ptvcursor_tree(cursor) != NULL check,
> after adding the subtree cursor ?

Maybe you can give me an example of what you guys would like to see. For
example by manually fixing handle_ButtonTemplateMessage (which is a simpler
example than UpdateCapabilitiesV3Message.

Would the 'tvb size guard' fix the issue ?

handle_ButtonTemplateMessage(ptvcursor_t *cursor, packet_info * pinfo _U_)
{
  guint32 totalButtonCount = 0;
  { /* start struct : buttonTemplate / size: 14 */
    ptvcursor_add_text_with_subtree(cursor, SUBTREE_UNDEFINED_LENGTH,
ett_skinny_tree, "buttonTemplate");
    ptvcursor_add(cursor, hf_skinny_buttonOffset, 4, ENC_LITTLE_ENDIAN);
    ptvcursor_add(cursor, hf_skinny_buttonCount, 4, ENC_LITTLE_ENDIAN);
    totalButtonCount = tvb_get_letohl(ptvcursor_tvbuff(cursor),
ptvcursor_current_offset(cursor));
    ptvcursor_add(cursor, hf_skinny_totalButtonCount, 4, ENC_LITTLE_ENDIAN);
    { /* start struct : definition / size: 2 */
      guint32 counter_2 = 0;
      ptvcursor_add_text_with_subtree(cursor, SUBTREE_UNDEFINED_LENGTH,
ett_skinny_tree, "definition [ref: totalButtonCount = %d, max:42]",
totalButtonCount);
      if (hdr_data_length - ptvcursor_current_offset(cursor) >=
(totalButtonCount * 2) && totalButtonCount <= 42) { /* tvb size guard */
        for (counter_2 = 0; counter_2 < 42; counter_2++) {
          if (counter_2 < totalButtonCount) {
            ptvcursor_add_text_with_subtree(cursor, SUBTREE_UNDEFINED_LENGTH,
ett_skinny_tree, "definition [%d / %d]", counter_2 + 1, totalButtonCount);
            ptvcursor_add(cursor, hf_skinny_instanceNumber, 1,
ENC_LITTLE_ENDIAN);
            ptvcursor_add(cursor, hf_skinny_buttonDefinition, 1,
ENC_LITTLE_ENDIAN);
          } else {
            ptvcursor_advance(cursor, 2);
          }
          ptvcursor_pop_subtree(cursor);
        } /* end for loop tree: definition */
      } /* end tvb size guard */
      ptvcursor_pop_subtree(cursor);
    } /* end struct: definition */
    ptvcursor_pop_subtree(cursor);
  } /* end struct: buttonTemplate */
}

---

      You are receiving this mail because:

• You are watching all bug changes.