do NOT close fp in pcap_close call when the fp obtained via pcap_fopen_offline

[crackevil]

• This is not a security issue (See first line).

libpcap version: 1.10.4 on windows.

calling pcap_close will close the fp which opened before pcap_fopen_offline.
the pcap_dump_close routine has the same issue.

[guyharris]

calling pcap_close will close the fp which opened before pcap_fopen_offline.

Yes, it will, and that behavior dates back to libpcap 0.9, when pcap_fopen_offline() was introduced.

Changing it would break the API and ABI contracts that libpcap offers to its callers; that could, for example, leak the FILE * and its FD in programs that expect pcap_close() to close the FILE *.

If there is code that uses libpcap and nees to use the FILE * after pcap_close() is called, we would have to introduce another routine that will flag the pcap_t as "don't close the FILE *".

(When a FILE * opened with fdopen() is closed, the FD that was provided to fdopen() is closed - "The fclose() function shall perform the equivalent of a close() on the file descriptor that is associated with the stream pointed to by stream." - which is an analogous behavior.)

[guyharris]

I will update the pcap_close() man page to explicitly state that the FILE * will be closed if the pcap_t corresponds to a savefile.

[crackevil]

thanks~
indeed, this behavior will impact old code. it's fine if the behavior keep consistent.

[guyharris]

I've updated the documentation.

I've also changed pcap_dump_close() to flush, but not close, the FILE * if it's stdout, just as pcap_close() doesn't close a savefile's FILE * if it's stdin.

[crackevil]

maybe new routines like pcap_close_without_fp will be more helpful

[infrastation]

One potential way to do this would be using a new function generalised from #1374 such as pcap_set_shutdown_mode() or even something such as pcap_set_flags(p, pcap_get_flags(p) | PCAP_CLOSE_LEAVE_FILES_OPEN).

[guyharris]

maybe new routines like pcap_close_without_fp will be more helpful

Where is this useful?

And, if it is, it should probably be specified at open time, not close time; pcap_close_without_fp() wuld not be useful for closing something opened with pcap_open_offline(), as that would leak a FILE *.

[crackevil]

let the code calling libpcap take the responsibility of managing the FILE * . of course pcap_close_without_fp() should NOT be called if pcap_open_offline() call earilier.

[guyharris]

let the code calling libpcap take the responsibility of managing the FILE * .

Either

1. the code was written to assume that the FILE * will be closed by pcap_close(), i.e., the behavior is what it has been since pcap_fopen_offline() was introduced, in which case it can't take that responsibility

or

2. the code wants the FILE * to remain open after pcap_close() is called, for some reason (what are the reasons why this is useful?), in which case the API could be modified either to allow it to request that at open time or at close time.

Doing so at open ime is preferable, for the reasons I mentioned.

of course pcap_close_without_fp() should NOT be called if pcap_open_offline() call earilier.

That imposes a requirement on the programmer to remember which pcap_ts were opened in that fashion.

At the cost of one flag in the data to which the pcap_t pointer points (which is a pcap_t structure, immediately followed by a structure whose contents depend on what type of pcap_t this is), a new routine to specify it at open time would avoid having to keep track of two different types of close routine. I.e., the programmer has to choose a routine in either case; one case allows a leak (if they call pcap_close_without_fp() on a pcap_t opened with pcap_open_offline()), and the other case doesn't.

[infrastation]

@crackevil, if you see a problem and expect a solution, please explain the use case in detail.

[crackevil]

in my case, I want libpcap read pcap file content from a pipe opened by other component. since libpcap did not open the FILE* so it should not close it. the action of the write end of the pipe is not predicable when libpcap close the FILE* of the read end of the pipe.
this is the use case.
under logic perspective, a library should take only limited responsibility, not all. if a piece of code open a FILE* itself, but expect libpcap to close the FILE*, it's totally logic error. explicitly calling a routine like pcap_close_without_fp() makes the responsibility very clear
who open, who close. old code has no impact at all. who call pcap_close_without_fp() obviously know that the FILE* is kept opened to let the caller handle it

[infrastation]

Thank you.

[guyharris]

in my case, I want libpcap read pcap file content from a pipe opened by other component. since libpcap did not open the FILE* so it should not close it.

Yes, popen() is one use case, as you need to close it with pclose() (to clean up the piped-from process and any memory used to keep track of that process).

the action of the write end of the pipe is not predicable when libpcap close the FILE* of the read end of the pipe.

So presumably the process reading from the pipe may stop reading and close the pcap_t before the other process closes the pipe.

who call pcap_close_without_fp() obviously know that the FILE* is kept opened to let the caller handle it

And whoever calls pcap_fopen_offline_noclose() also knows that the FILE * is being kept open to let the caller handle it.

But it's not obvious that the programmer will get it right.

The question is which produces code that's clearer to read (associating the "don't close the FILE * behavior with the open, which may be near the open of the FILE *, or with the close, which may be near the close of the FILE *) and which is more likely to avoid people calling pcap_close_leave_fp_open() on a pcap_t where leaving the FILE * open leaks it.