Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pcapreader() does not work at specific packets and memory usage continues to increase #1407

Closed
no-okui opened this issue May 10, 2018 · 13 comments
Assignees
Labels

Comments

@no-okui
Copy link

no-okui commented May 10, 2018

Scary version: scapy-2.4.0.dev149
Python version: Python 3.6.5
OS: Ubuntu 16.04.03 LTS

I try to read the attached pcap file using Pcapreader(), but Pcapreader() does not work after read a packet which timestamp is 1523958716.205101, and memory usage increases, finally, the process forcibly terminates.

from scapy.all import *
with PcapReader('test.pcap') as pkts:
    for pkt in pkts:
        print(str(pkt.time) + ': ' + str(len(pkt)))

output

1523958716.093239: 82
1523958716.14647: 82
1523958716.159972: 98
1523958716.18894: 90
1523958716.201178: 90
1523958716.205101: 90

I also try to read the attached pcap file using dpkt and dpkt succeed in read the file.

import dpkt
with open('test.pcap', "rb") as f:
    pcr = dpkt.pcap.Reader(f)
    for t, buf in pcr:
        print(str(t) + ': ' + str(len(buf)))
        eth = dpkt.ethernet.Ethernet(buf)

        ip = eth.data
        src = ip.src
        dst = ip.dst
        src_a = socket.inet_ntoa(src)
        dst_a = socket.inet_ntoa(dst)
        print("Sorce IP :%s" % src_a)
        print("Dest IP :%s" % dst_a)

output

1523958716.093239: 82
Sorce IP :210.196.129.140
Dest IP :8.8.8.8
1523958716.14647: 82
Sorce IP :210.196.129.140
Dest IP :8.8.8.8
1523958716.159972: 98
Sorce IP :8.8.8.8
Dest IP :210.196.129.140
1523958716.18894: 90
Sorce IP :210.196.129.140
Dest IP :13.113.26.230
1523958716.201178: 90
Sorce IP :210.196.129.140
Dest IP :13.113.26.230
1523958716.205101: 90
Sorce IP :210.196.129.140
Dest IP :13.113.26.230
1523958716.209078: 90
Sorce IP :210.196.129.140
Dest IP :13.113.26.230
1523958716.213085: 90
Sorce IP :210.196.129.140
Dest IP :13.113.26.230


1523958716.453802: 114
Sorce IP :121.40.147.12
Dest IP :210.196.129.140

test.pcap.zip

@p-l- p-l- added the bug label May 11, 2018
@p-l- p-l- self-assigned this May 11, 2018
@p-l-
Copy link
Member

p-l- commented May 11, 2018

Thanks a lot for reporting this.

@p-l-
Copy link
Member

p-l- commented May 11, 2018

You may want to try #1409 patch.

@nluedtke
Copy link

For some reason, this was assigned CVE-2019-1010142.

@gpotter2
Copy link
Member

gpotter2 commented Jul 22, 2019

@guedou @p-l- it's unexpected knowing this was patched and packed in 2.4.2 :/

@guedou
Copy link
Member

guedou commented Jul 22, 2019

I think that this is a consequence of really aggressive communication from Imperva, see https://www.imperva.com/blog/scapy-sploit-python-network-tool-is-vulnerable-to-denial-of-service-dos-attack-cve-pending/

The maintainers were never informed of this ...

@guedou
Copy link
Member

guedou commented Jul 22, 2019 via email

@gpotter2
Copy link
Member

We might wanna try using https://github.com/secdev/scapy/network/alerts
to disclaim that this issue was fixed ages ago.

@greysteil
Copy link

I work on GitHub's security workflows team and am looking into how we can make security alerts work better for maintainers. I'd love your feedback if you have 5 mins?

We pick up new vulnerabilities when maintainers submit a Security Advisory (in the security tab on this repo) or when they appear in the NVD feed - in this case it's CVE-2019-1010142 that brought this to our attention. We then have a manual curation process (the NVD feed isn't structured enough to process automatically, and can contain spurious reports). In this case we've picked up that v2.4.1 onwards are patched and added that to our alerts, which also power automated security fix PRs like this one.

I'd love your feedback on the process. What could we be doing better, from your perspective as maintainers? If you'd have been able to use "Maintainer Security Advisories" to publicise this vulnerability back in May 2018 would you have? What would make that feature useful to you in future, and how would you like us to handle vulnerabilities we're alerted of via the NVD feed?

Any thoughts really appreciated - we're trying to make the process better for everyone, but I appreciate there may be some sharp edges at the moment. You can email me on [email protected] if you'd like to discuss anything privately.

@gpotter2
Copy link
Member

Hi, I don't have much to say about that. Here are a few thoughts:

  1. I would have expected this form to show up on the "new" page, rather than in some hidden dropdown. I had no idea it existed before your post.
    image
  2. It would have been niced if an advisory on this page had been created by GitHub when you started advertising for the issue. We (maintainers) have not been contacted by either Imperva (who aggressively harassed Twitter with this issue), nor Github (except by you) that they were rolling out those warnings.
  3. Personally, I think the initiative taken by GitHub to create this security-disclaimer service is great. I'm not sure we realized at the time the noise this issue would make (Scapy doesn't tend to be used in production in other places than unit tests) and ended up rushing the release of 2.4.1/2.4.2 (they are buggy releases). I (we?) would probably have used it if it was clearer (see point 1)
  4. If I understand this correctly, only users that have enabled "dependabot-preview" get the warnings. I know GitHub tracks all dependencies very well (see Used By tab), so it might be a good idea to warn all maintainers. Just a thought

@greysteil
Copy link

Really helpful, thanks @gpotter2.

If I understand this correctly, only users that have enabled "dependabot-preview" get the warnings.

Folks who have dependabot-preview or automated security fixes enabled will have got PRs, whilst folks with vulnerability alerts enabled will have got alert emails. We're rolling out automated security fixes to everyone and will eventually decommission Dependabot Preview (once all its functionality is available within GitHub) which should reduce the confusion in future.

It would have been niced if an advisory on this page had been created by GitHub when you started advertising for the issue

We're hearing this a lot and want to do something about it! :octocat:

@nluedtke
Copy link

@greysteil As someone who both uses the PRs and the Advisory's i have some ideas but mainly around the discovery and notification process. As many developers simply never know when a CVE is filed against their project. I'll shoot you an email when I get less busy.

@guedou
Copy link
Member

guedou commented Jul 24, 2019

@greysteil thanks for reaching us!

To complete @gpotter2 comments, I think that it would be really helpful to contact maintainers on github before/while sending notifications to other repositories. I discovered that you were sending notifications because on my project depends on Scapy.

Also, it might be useful to have a list of projects/contacted and opened PR in the security tab.

I will be at BHUSA next month. We could follow the discussion if you attend too.

@greysteil
Copy link

Thanks @guedou. I'm hearing the "contact us first" feedback from everyone I ask for feedback, so we'll almost certainly be implementing that.

Sadly won't be at BHUSA - have an awesome time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants