Check added to PcapNg processing

[guedou]

This PR fixes issues discovered by oss-fuzz.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 59.37500% with 13 lines in your changes are missing coverage. Please review.

Project coverage is 82.21%. Comparing base (ac3d5bb) to head (371aaf6). Report is 2 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #4373   +/-   ##
=======================================
  Coverage   82.20%   82.21%           
=======================================
  Files         353      353           
  Lines       83529    83607   +78     
=======================================
+ Hits        68662    68734   +72     
- Misses      14867    14873    +6     

Files	Coverage Δ
scapy/utils.py	73.01% <59.37%> (-0.23%)	:arrow_down:

... and 6 files with indirect coverage changes

[evverx]

I apologize for bringing this up here but since it's related to OSS-Fuzz I wonder if it would be OK if I opened https://github.com/google/oss-fuzz/pull/9629 again?

[guedou]

@evverx I am fine with the proposed PR that loads all layers. However, I will prefer to keep the reports to the Scapy maintainers only.

[evverx]

Fair enough. I fuzz scapy elsewhere so I don't need access to bug reports on OSS-Fuzz (though it would make it easier to filter out duplicates, comment on issues like https://github.com/secdev/scapy/issues/3145#issuecomment-1454702817 reproducible by that fuzz target in certain environments only and so on). I'll open a PR without my email address then.

[guedou]

The others maintainers might have a different opinion.

[gpotter2]

I'm personally fine with @evverx having their email there.

Two reasons:

• oss-fuzz has currently never reported anything worth worrying about (due to the nature of Scapy, we can't really have anything much more serious than a DoS), meaning it's actually not that big of a deal.
• @evverx has provided valuable contributions to Scapy, and I believe they're asking in good faith.

[p-l-]

@guedou Do you think it would be interesting to add some problematic files as test cases here? Or do we consider oss-fuzz to be enough?

I agree w/ @gpotter2 re. adding @evverx mail.

[guedou]

One test case is small (~100 bytes), while the two others one are big (1k+ bytes). I could try to reduce the big ones if you think that it is worth the effort.

[p-l-]

Not necessarily, that was more a question than a change request to your PR!

[guedou]

@gpotter2 I managed to strip down the test cases to 12, 100 and 140 bytes. If that's OK for you, I will add the corresponding unit tests.

[evverx]

Do you think it would be interesting to add some problematic files as test cases here?

FWIW It should be possible to turn on CIFuzz: https://google.github.io/oss-fuzz/getting-started/continuous-integration/. It downloads corpora accumulated by OSS-Fuzz (including test cases that triggered issues in the past) and runs fuzz targets with them. It doesn't catch all the issues but it should be enough to catch most regressions when PRs are opened. Though it looks like OSS-Fuzz is updating the toolchains, interpreters and some other things so I'd wait for them to finish just in case to avoid running into things like https://github.com/google/oss-fuzz/issues/11886.

[evverx]

(I opened https://github.com/google/oss-fuzz/pull/11912. It should hopefully make the regression test from https://github.com/secdev/scapy/pull/4378 a bit more useful)