[Wireshark-bugs] [Bug 12826] usage http-tcp dissector from lua dissector lead to Segmentation fault

[bugzilla-daemon]

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=12826

Peter Wu  changed:

   What|Removed |Added

   See Also||https://bugs.wireshark.org/
   ||bugzilla/show_bug.cgi?id=14
   ||293

-- 
You are receiving this mail because:
You are watching all bug changes.___
Sent via:Wireshark-bugs mailing list 
Archives:https://www.wireshark.org/lists/wireshark-bugs
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-bugs
 mailto:wireshark-bugs-requ...@wireshark.org?subject=unsubscribe

[Wireshark-bugs] [Bug 12826] usage http-tcp dissector from lua dissector lead to Segmentation fault

[bugzilla-daemon]

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=12826

Michael Mann  changed:

   What|Removed |Added

 Resolution|--- |FIXED
 Status|CONFIRMED   |RESOLVED

--- Comment #14 from Michael Mann  ---
This seems to just be able crashing when calling "HTTP over TCP" dissector
because of NULL data.   That has been fixed, so closing bug.

-- 
You are receiving this mail because:
You are watching all bug changes.___
Sent via:Wireshark-bugs mailing list 
Archives:https://www.wireshark.org/lists/wireshark-bugs
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-bugs
 mailto:wireshark-bugs-requ...@wireshark.org?subject=unsubscribe

[Wireshark-bugs] [Bug 12826] usage http-tcp dissector from lua dissector lead to Segmentation fault

[bugzilla-daemon]

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=12826

Stig Bjørlykke  changed:

   What|Removed |Added

 CC||s...@bjorlykke.org

--- Comment #13 from Stig Bjørlykke  ---
We cannot have a crash when running Lua scripts so adding a check for NULL
pointer is better than nothing.  This was fixed in bug 13457.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Sent via:Wireshark-bugs mailing list 
Archives:https://www.wireshark.org/lists/wireshark-bugs
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-bugs
 mailto:wireshark-bugs-requ...@wireshark.org?subject=unsubscribe

[Wireshark-bugs] [Bug 12826] usage http-tcp dissector from lua dissector lead to Segmentation fault

[bugzilla-daemon]

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=12826

--- Comment #12 from Stig Bjørlykke  ---
*** Bug 13457 has been marked as a duplicate of this bug. ***

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Sent via:Wireshark-bugs mailing list 
Archives:https://www.wireshark.org/lists/wireshark-bugs
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-bugs
 mailto:wireshark-bugs-requ...@wireshark.org?subject=unsubscribe

[Wireshark-bugs] [Bug 12826] usage http-tcp dissector from lua dissector lead to Segmentation fault

[bugzilla-daemon]

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=12826

--- Comment #11 from Guy Harris  ---
(In reply to Peter Wu from comment #8)
> Comment 5 uses an approach that seems quite common for Lua dissectors (I
> think it is also documented in an example Lua dissector): obtain old handle,
> override dissectors, call old dissector and act on it.
> 
> I think that https://code.wireshark.org/review/16176 is sufficient for
> correctness (i.e. not crash on missing data), but unfortunately loses the
> possibility to propagate the end-of-stream flag from the TCP layer to HTTP.
> 
> In C dissectors, we rely on code review and conventions to avoid illegal
> "data" parameters (though we do have type confusion problems at times).
> 
> We cannot rely on the Lua dissector not to pass garbage. Currently it always
> passes a NULL data parameter which is handled gracefully by at least:
> modbus (mbtcp), ethertype, wlan (ieee80211). (Searched for
> call_dissector_with_data and looked at a random sample).
> 
> Maybe we should drop this data parameter and use p_add_proto_data:

...which is a mechanism for persistent data attached to packets; I'd prefer not
to use it for data that can be generated by the calling dissector on the fly.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Sent via:Wireshark-bugs mailing list 
Archives:https://www.wireshark.org/lists/wireshark-bugs
Unsubscribe: https://wireshark.org/mailman/options/wireshark-bugs
 mailto:wireshark-bugs-requ...@wireshark.org?subject=unsubscribe

[Wireshark-bugs] [Bug 12826] usage http-tcp dissector from lua dissector lead to Segmentation fault

[bugzilla-daemon]

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=12826

--- Comment #10 from Peter Wu  ---
(In reply to Michael Mann from comment #9)
> (In reply to Peter Wu from comment #8)
> > Maybe we should drop this data parameter and use p_add_proto_data:
> 
> I would rather rewrite the dissector function signature (to have "data
> pointer + type") than use p_add_proto_data.  It makes code flow MUCH harder
> to follow (and easier to create bugs as a result).  It was one of the
> reasons I tried to rid dissectors of using pinfo->private_data (and other
> members than have since been removed).

If a dissector documents what data it provides or requires, then how would it
be harder to follow the code flow? The data is per-packet and you are supposed
to set the field before invoking the subdissector.

> For this particular bug, I would prefer that http have 2 function signatures
> - one that expects TCP data and one that doesn't.  Then both functions would
> call a "common" function where necessary TCP data would be passed in
> (defaulted in function with NULL data)

HTTP has already multiple signatures (http-over-tls, http-over-sctp,
http-over-tcp and just "http"). IIRC only the "http-over-tcp" uses the data
parameter at the moment. Alternative functions will not solve the problem of
Lua being able to obtain a dissector handle and then invoking that with a NULL
data parameter.

For this case where the "tcp" dissector always provides the data, we could add
this special case in the Lua dissector:

 If the original dissector table matches the one of the subdissector, pass the
data parameter. E.g. the original http dissector is registered on "tcp.port",
the new dissector function is likely registered to this as well, so propagate
the tcp_info. If the new dissector function was registered through
"usb.product" (for example), then just pass NULL.

This does not help though in cases like the modbus (mbtcp) dissector since that
is not registered through a dissector table. The current APIs unfortunately do
not have the information on the expected type.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Sent via:Wireshark-bugs mailing list 
Archives:https://www.wireshark.org/lists/wireshark-bugs
Unsubscribe: https://wireshark.org/mailman/options/wireshark-bugs
 mailto:wireshark-bugs-requ...@wireshark.org?subject=unsubscribe

[Wireshark-bugs] [Bug 12826] usage http-tcp dissector from lua dissector lead to Segmentation fault

[bugzilla-daemon]

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=12826

--- Comment #9 from Michael Mann  ---
(In reply to Peter Wu from comment #8)
> Maybe we should drop this data parameter and use p_add_proto_data:

I would rather rewrite the dissector function signature (to have "data pointer
+ type") than use p_add_proto_data.  It makes code flow MUCH harder to follow
(and easier to create bugs as a result).  It was one of the reasons I tried to
rid dissectors of using pinfo->private_data (and other members than have since
been removed).

For this particular bug, I would prefer that http have 2 function signatures -
one that expects TCP data and one that doesn't.  Then both functions would call
a "common" function where necessary TCP data would be passed in (defaulted in
function with NULL data)

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Sent via:Wireshark-bugs mailing list 
Archives:https://www.wireshark.org/lists/wireshark-bugs
Unsubscribe: https://wireshark.org/mailman/options/wireshark-bugs
 mailto:wireshark-bugs-requ...@wireshark.org?subject=unsubscribe

[Wireshark-bugs] [Bug 12826] usage http-tcp dissector from lua dissector lead to Segmentation fault

[bugzilla-daemon]

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=12826

Peter Wu  changed:

   What|Removed |Added

 CC||g...@alum.mit.edu,
   ||hadri...@yahoo.com,
   ||pe...@lekensteyn.nl

--- Comment #8 from Peter Wu  ---
Comment 5 uses an approach that seems quite common for Lua dissectors (I think
it is also documented in an example Lua dissector): obtain old handle, override
dissectors, call old dissector and act on it.

I think that https://code.wireshark.org/review/16176 is sufficient for
correctness (i.e. not crash on missing data), but unfortunately loses the
possibility to propagate the end-of-stream flag from the TCP layer to HTTP.

In C dissectors, we rely on code review and conventions to avoid illegal "data"
parameters (though we do have type confusion problems at times).

We cannot rely on the Lua dissector not to pass garbage. Currently it always
passes a NULL data parameter which is handled gracefully by at least:
modbus (mbtcp), ethertype, wlan (ieee80211). (Searched for
call_dissector_with_data and looked at a random sample).

Maybe we should drop this data parameter and use p_add_proto_data:
 - as provider: the tcp dissector can provide "tcpinfo" to the interested
subdissectors.
 - as consumer: the mbtcp dissector requires a "packet_type" parameter from
"consumers" that invoke this dissector (cip, ecmp).

The type is more explicit in this case (defined by the protocol ID and the
protocol-specific key).

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Sent via:Wireshark-bugs mailing list 
Archives:https://www.wireshark.org/lists/wireshark-bugs
Unsubscribe: https://wireshark.org/mailman/options/wireshark-bugs
 mailto:wireshark-bugs-requ...@wireshark.org?subject=unsubscribe

[Wireshark-bugs] [Bug 12826] usage http-tcp dissector from lua dissector lead to Segmentation fault

[bugzilla-daemon]

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=12826

--- Comment #7 from Michael Mann  ---
(In reply to Alexis La Goutte from comment #6)
> Michael coming from https://code.wireshark.org/review/12984

Potential solution provided here:
https://code.wireshark.org/review/16176

Not sure I like the NULL check because I'd prefer different dissector functions
if their "signature" is different (taking data vs not taking data), but it is
the "simplest" fix.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Sent via:Wireshark-bugs mailing list 
Archives:https://www.wireshark.org/lists/wireshark-bugs
Unsubscribe: https://wireshark.org/mailman/options/wireshark-bugs
 mailto:wireshark-bugs-requ...@wireshark.org?subject=unsubscribe

[Wireshark-bugs] [Bug 12826] usage http-tcp dissector from lua dissector lead to Segmentation fault

[bugzilla-daemon]

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=12826

Alexis La Goutte  changed:

   What|Removed |Added

 Status|UNCONFIRMED |CONFIRMED
 CC||mman...@netscape.net
 Ever confirmed|0   |1

--- Comment #6 from Alexis La Goutte  ---
Michael coming from https://code.wireshark.org/review/12984

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Sent via:Wireshark-bugs mailing list 
Archives:https://www.wireshark.org/lists/wireshark-bugs
Unsubscribe: https://wireshark.org/mailman/options/wireshark-bugs
 mailto:wireshark-bugs-requ...@wireshark.org?subject=unsubscribe

[Wireshark-bugs] [Bug 12826] usage http-tcp dissector from lua dissector lead to Segmentation fault

[bugzilla-daemon]

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=12826

jflevesque  changed:

   What|Removed |Added

 CC||jflevesque.m...@gmail.com

--- Comment #5 from jflevesque  ---
Hi all,

First time poster. I have upgraded to 2.2.0 and am currently using a lua
dissector which replaces the HTTP dissector in the tcp table. When we compile
the lua dissector, we keep a reference to the HTTP dissector so we can still
use it to decode the HTTP header and get info like the fulluri, content length
and status code.

When we call the dissector, we now get an error message: "* (tshark.exe:18484):
WARNING **: Dissector bug, protocol HTTP, in packet 357:
STATUS_ACCESS_VIOLATION: dissector accessed an invalid memory address". This
causes Wireshark to crash.

Does this error have the same cause as ychislov reported? If so, should we
change how we use our dissector or is there a fix for the HTTP dissector?

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Sent via:Wireshark-bugs mailing list 
Archives:https://www.wireshark.org/lists/wireshark-bugs
Unsubscribe: https://wireshark.org/mailman/options/wireshark-bugs
 mailto:wireshark-bugs-requ...@wireshark.org?subject=unsubscribe

[Wireshark-bugs] [Bug 12826] usage http-tcp dissector from lua dissector lead to Segmentation fault

[bugzilla-daemon]

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=12826

--- Comment #4 from ychis...@trustwave.com ---
I should think about.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Sent via:Wireshark-bugs mailing list 
Archives:https://www.wireshark.org/lists/wireshark-bugs
Unsubscribe: https://wireshark.org/mailman/options/wireshark-bugs
 mailto:wireshark-bugs-requ...@wireshark.org?subject=unsubscribe

[Wireshark-bugs] [Bug 12826] usage http-tcp dissector from lua dissector lead to Segmentation fault

[bugzilla-daemon]

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=12826

Alexis La Goutte  changed:

   What|Removed |Added

 CC||alexis.lagou...@gmail.com

--- Comment #3 from Alexis La Goutte  ---
Need to add a check to see if the value is NULL

About Proxy Protocol, do you plan to convert to C dissector ? (i have a draft
of v1 proxy protocol but never finish)
And v2 protocol is binary (and will be more easy to write...)

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Sent via:Wireshark-bugs mailing list 
Archives:https://www.wireshark.org/lists/wireshark-bugs
Unsubscribe: https://wireshark.org/mailman/options/wireshark-bugs
 mailto:wireshark-bugs-requ...@wireshark.org?subject=unsubscribe

[Wireshark-bugs] [Bug 12826] usage http-tcp dissector from lua dissector lead to Segmentation fault

[bugzilla-daemon]

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=12826

--- Comment #2 from ychis...@trustwave.com ---
Created attachment 14882
  --> https://bugs.wireshark.org/bugzilla/attachment.cgi?id=14882=edit
core backtarce output

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Sent via:Wireshark-bugs mailing list 
Archives:https://www.wireshark.org/lists/wireshark-bugs
Unsubscribe: https://wireshark.org/mailman/options/wireshark-bugs
 mailto:wireshark-bugs-requ...@wireshark.org?subject=unsubscribe

[Wireshark-bugs] [Bug 12826] usage http-tcp dissector from lua dissector lead to Segmentation fault

[bugzilla-daemon]

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=12826

--- Comment #1 from ychis...@trustwave.com ---
Created attachment 14881
  --> https://bugs.wireshark.org/bugzilla/attachment.cgi?id=14881=edit
proxy protocol v2 pcap

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Sent via:Wireshark-bugs mailing list 
Archives:https://www.wireshark.org/lists/wireshark-bugs
Unsubscribe: https://wireshark.org/mailman/options/wireshark-bugs
 mailto:wireshark-bugs-requ...@wireshark.org?subject=unsubscribe