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

Adds tcpdump project #1757

Closed
wants to merge 4 commits into from
Closed

Adds tcpdump project #1757

wants to merge 4 commits into from

Conversation

catenacyber
Copy link
Contributor

@jonathanmetzman
Copy link
Contributor

Thanks!
Just get someone from the project to approve please.

@inferno-chromium
Copy link
Collaborator

@fxlb, @guyharris, @infrastation, @mcr - are you ok with fuzzing of tcpdump and finding stability/security bugs ? can you also look at the pull request to see if fuzzer code looks ok ?

RUN apt-get update && apt-get install -y make cmake flex bison
RUN git clone --depth=1 https://github.com/the-tcpdump-group/libpcap.git libpcap
#TODO use main repo
RUN git clone --depth=1 --branch fuzz https://github.com/catenacyber/tcpdump.git tcpdump
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be the original tcpdump repo, if there are custom patches, please add a diff file and then apply in build.sh

MAINTAINER security@tcpdump.org
RUN apt-get update && apt-get install -y make cmake flex bison
RUN git clone --depth=1 https://github.com/the-tcpdump-group/libpcap.git libpcap
#TODO use main repo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Space between # and "TODO".

# export other associated stuff
cd ..
cp fuzz/fuzz_*.options $OUT/
#builds corpus
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Space between # and "builds"

@infrastation
Copy link
Contributor

Thank you for asking, this is being considered.

@catenacyber
Copy link
Contributor Author

@infrastation let me know if you want to talk about this.
Did you get my mail at security@tcpdump.org about the buffer overread ?

@infrastation
Copy link
Contributor

I have checked and all recent reports look answered, so if you didn't get a response please tell the date when you sent or forward the message once again.

@catenacyber
Copy link
Contributor Author

I sent it on the 25th of August, 20:47 French time
I got an automatic answer

Your mail to 'tcpdump-security' with the subject

   Heap buffer over read

Is being held until the list moderator can review it for approval.

The reason it is being held:

   Post by non-member to a members-only list

@infrastation
Copy link
Contributor

@mcr, could you unblock this please?

@mcr
Copy link

mcr commented Aug 31, 2018 via email

@infrastation
Copy link
Contributor

On behalf of the tcpdump group: regarding tcpdump fuzzing, this would be a good thing to do, but we would like to put this request on hold for now because of internal work that needs to be finished first (this may take another few months). More follow-ups later.

@Dor1s
Copy link
Contributor

Dor1s commented Dec 27, 2018

@catenacyber @infrastation should we proceed with this now?

@infrastation
Copy link
Contributor

No, please hold it.

cmake ..
make

# build fuzz targets
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather see this be done as part of the tcpdump build process, so that the project can add fuzz targets without having to push a change to build.sh.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If that helps, the fuzz targets are built with tcpdump as well, but with a different stand alone driver (onefile.c)

so that the project can add fuzz targets without having to push a change to build.sh
This is indeed a useful feature when the need arises

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's true that they are built as part of the tcpdump build process, but that is not useful to clusterfuzz. They have to be built during this process using the fuzzer library to be able to be fuzzed in the cluster.

I have a Makefile.in that can build either tcpdump-way or clusterfuzz-way, but I don't know how to do it in cmake. We can simply add the "build the fuzzers the clusterfuzz way" build.sh in /src/tcpdump/fuzz/build.sh and invoke that from here.

@catenacyber
Copy link
Contributor Author

ping @infrastation any news ?

@mcr
Copy link

mcr commented Jun 7, 2019

Progress is slower than desired, but there is progress. Please continue to hold off.

@fxlb
Copy link

fxlb commented Jun 10, 2019

Is the 90 days disclosure deadline configurable or mandatory ?

@Dor1s
Copy link
Contributor

Dor1s commented Jun 10, 2019

It's mandatory, but can be extended for two more weeks under some circumstances: https://github.com/google/oss-fuzz/blob/master/README.md#bug-disclosure-guidelines

@fxlb
Copy link

fxlb commented Jun 11, 2019

It's mandatory

Thus adding tcpdump project to oss-fuzz makes we do not control the deadlines.

It should be better to have our own oss-fuzz instance to keep the control.

@inferno-chromium
Copy link
Collaborator

@fxlb - 90 days is quite a long period, we havent had this issue with currently integrated 200+ projects. Also, with https://github.com/google/oss-fuzz/blob/master/docs/ideal_integration.md, you will find regressions quickly in-house itself. So over time, you will not have any stable impacting bugs with continuous fuzzing at scale.

@evverx
Copy link
Contributor

evverx commented Jul 12, 2019

It's mandatory

According to #2590 (comment), it seems the standard disclosure policy isn't always enforced. The jsonnet project seems to have been waiting for false positives (not only with MSan but with ASan and UBsan too) to pop up since December 2018 bypassing the policy altogether. Looks like a double standard to me.

we havent had this issue with currently integrated 200+ projects

I suspect those who decline to abide by the policy simply aren't integrated so it's hard to tell how many projects are unhappy about it. I remember some blockchain folks saying on Twitter that they wouldn't consider OSS-Fuzz due to not having control over deadlines.

@inferno-chromium
Copy link
Collaborator

@evverx - we are humans and we try to enforce the policy as best as we can. i am removing the jsonnet project now as there was no followup and this is not the intended way of use.

@inferno-chromium
Copy link
Collaborator

@evverx - note that some projects will still keep experimental=true such as libsass, FreeImage, since it is a googler trying to find the library developer. +cc @stefanbucur @Google-Autofuzz. Our disclosure policies only apply when we have a library developer in cc list/ owner list and they can actually handle the deadline.

@evverx
Copy link
Contributor

evverx commented Jul 12, 2019

Our disclosure policies only apply when we have a library developer in cc list/ owner list and they can actually handle the deadline.

In theory, it means that it's possible for a person integrating a project to mark it "experimental" to hide it from the standard disclosure policy and be a liaison between OSS-Fuzz and the developers who can fix bugs as they see fit. Sounds good to me too.

@inferno-chromium
Copy link
Collaborator

@evverx - this was not the intent and we won't be allowing misuse of this.

@evverx
Copy link
Contributor

evverx commented Jul 13, 2019

@inferno-chromium I'm pretty sure it has never been misused. What I was trying to say is that as long as there're loopholes like that the disclosure is really optional. Given that the point is to find bugs and get them fixed, I think it would make sense to allow projects to, I don't know, temporarily bypass it.

@evverx
Copy link
Contributor

evverx commented Jul 13, 2019

Regarding the projects whose developers can't be found, it's worth mentioning that once a project is integrated, literally anyone can unleash as many resources on their infrastructure as they can because it's easy so I frankly don't understand what purpose "experimental: True" serves. Some shady people view bugs, the googlers supposedly looking for the developers view them as well (by the way, what happens when bugs are found?) and apparently only the public is absolutely unaware of what's going on.

I'd say that the policy should be either always enforced no matter what (by removing "experimental: True" altogether) or it should be possible to turn it off.

@evverx
Copy link
Contributor

evverx commented Jul 13, 2019

by the way, what happens when bugs are found?

Hm, apparently, some bugs are fixed git/git@98552f252ad4a86573d7566 but at the same time the git project has been protected by "experimental: True" since November 2018: 89c53fe. Interestingly, the author of the fix and the maintainer of the project on OSS-Fuzz is the same person. I, frankly, don't know what to say.

Anyway, I apologize for derailing the conversation. I hope tcpdump will be integrated.

@inferno-chromium
Copy link
Collaborator

by the way, what happens when bugs are found?

Hm, apparently, some bugs are fixed git/git@98552f2 but at the same time the git project has been protected by "experimental: True" since November 2018: 89c53fe. Interestingly, the author of the fix and the maintainer of the project on OSS-Fuzz is the same person. I, frankly, don't know what to say.

This was a mistake and was already corrected in #2596

Anyway, I apologize for derailing the conversation. I hope tcpdump will be integrated.

Yes please, if you have more comments, please open a new bug.

@evverx
Copy link
Contributor

evverx commented Jul 13, 2019

This was a mistake and was already corrected in #2596

It would have been corrected if the bugs (especially the ones apparently fixed silently in jsonnet) had gone through the disclosure policy retrospectively. Flipping the bit after more than half a year of fuzzing isn't enough to say that everything is alright in my opinion.

@catenacyber
Copy link
Contributor Author

ping @fxlb @mcr ? Do you want to make progress on this as is done with HackerOne https://hackerone.com/ibb-data ?

@mcr
Copy link

mcr commented Feb 15, 2020

We'd like to get another release under our belt before we can commit to 90 day fixes.

@TravisBuddy
Copy link

Travis tests have failed

Hey @catenacyber,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

TravisBuddy Request Identifier: b9e44a50-5193-11ea-8f4e-3dd8abb23602

@TravisBuddy
Copy link

Hey @catenacyber,
Your changes look good to me!

View build log

TravisBuddy Request Identifier: 836e8cb0-51a7-11ea-8f4e-3dd8abb23602

@Dor1s
Copy link
Contributor

Dor1s commented Feb 18, 2020

We'll be happy to land this once upstream maintainers approve and the repo url is fixed in the Dockerfile.

@inferno-chromium
Copy link
Collaborator

@infrastation - would be interested in OSS-Fuzz integration ? We would love to have this conversation restarted and have tcpdump in OSS-Fuzz. Please also see our list of integrated projects - https://github.com/google/oss-fuzz/tree/master/projects.

@fxlb
Copy link

fxlb commented Apr 17, 2020

I don't want

  1. a mandatory 90 days disclosure deadline (even extended for two more weeks),
  2. to be required to have a google account to be able to login.

@JulianVolodia
Copy link
Contributor

Hey @fxlb . Nice to meet you. I wanted to integrate that, because I am one of your code users. Appreciate.
As I know being CC in project file, will make you being infromed about bugs, and hmm... not having the Google account? But maybe I missed sth out there.

After all, other could find these bugs, sooner or later, and nobody will force them to publish them nor show it to you. I am really curious about your point of view so if we could talk a moment I will really appreciate. Have a good day.

@debrouxl
Copy link

debrouxl commented Sep 4, 2021

The initial version of this PR is over 3 years old by now, and the status quo means that users of this popular project (FWIW, currently around the 2000th spot on Debian pocon by_inst, with ~1/5 of the 200+K computers reporting to popcon equipped with tcpdump) potentially remain vulnerable to security-relevant bugs not yet found by earlier fuzzing work, which already found many issues.
I'm sympathetic to open-source maintainers working unpaid on their free time - most of them do that, I do and I've worked with a significant number of others who do. Still, it's sad that this PR remains in limbo over maintainer acceptance of the standard 90-day disclosure timeline, on a high-profile project.
Is there anything bystanders like me can do to help ?

@mcr
Copy link

mcr commented Sep 4, 2021

The initial version of this PR is over 3 years old by now, and the status quo means that users of this popular project (FWIW, currently around the 2000th spot on Debian pocon by_inst, with ~1/5 of the 200+K computers reporting to popcon equipped with tcpdump) potentially remain vulnerable to security-relevant bugs not yet found by earlier fuzzing work, which already found many issues.
I'm sympathetic to open-source maintainers working unpaid on their free time - most of them do that, I do and I've worked with a significant number of others who do. Still, it's sad that this PR remains in limbo over maintainer acceptance of the standard 90-day disclosure timeline, on a high-profile project.
Is there anything bystanders like me can do to help ?

  1. close issues in tcpdump github
  2. get Google and the other entities that are paying out bug-bounties to pay 5x for having submitted a fix.
  3. volunteer to help us do CVE tracking

I'm all for bug-bounties, but they have created an ecosystem of disinterested parties who just race to find obscure bugs. And then not bother to figure out what the root cause problem is, or how to actually exploit the bug. Usually, since they don't fuzz the actual parts of the packet that can be manipulated by outsiders, they find bugs in header parsing which are not exploitable. It takes days to find the problem, and they pester us for a CVE number, so they can collect their bounty.

@debrouxl
Copy link

debrouxl commented Sep 4, 2021

I see. I've never collected a bug bounty myself, and my CVE request over issue report ratio is tiny (all the more a small number of CVE numbers is normally enough to prod packagers for the major, well-behaved distros into upgrading the affected software), but both are clearly a part of the ecosystem which needs to be dealt with...

Besides the advantage of scale for finding some of the more interesting bugs, wouldn't the integration into OSS-Fuzz be a way to reduce at least the interaction with bug bounty hunters, if not CVE requests ?

I usually triage the bugs found by fuzzers (usually DoS or infoleak) before sending them to the respective maintainers, but seldom participate in fixing the code or CVE tracking... Most of the maintainers I've worked with fixed all reported issues, no matter the exploitability. Some of them refactored the code to improve the fuzzability, the execution efficiency and/or the coverage ratio.

@evverx
Copy link
Contributor

evverx commented Sep 4, 2021

it's sad that this PR remains in limbo over maintainer acceptance of the standard 90-day disclosure timeline, on a high-profile project.

@debrouxl FWIW there are other places where code can be fuzzed continuously without the OSS-Fuzz limitations. It's also possible to roll out fuzzing infrastructure manually. Usually it isn't free but I guess people (and/or companies) saying how important tcpdump is to them and willing to help to set up continuous fuzzing shouldn't have any problem with that.

wouldn't the integration into OSS-Fuzz be a way to reduce at least the interaction with bug bounty hunters, if not CVE requests ?

Given that there seems to be a bot or something like that that blindly assigns CVEs to issues OSS-Fuzz considers "vulnerabilities" (simply copy-pasting backtraces from the bug reports into the "description" field along the way) I'm not sure what exactly OSS-Fuzz reduces.

Just to clarify, I'm not a tcpdump maintainer. I just subscribed to this issue a long time ago.

@oliverchang oliverchang closed this Dec 1, 2022
@catenacyber
Copy link
Contributor Author

Hello @mcr would you be interested in this ? tcpdump (by release 4.9.3) fixed all the issues I knew of...

@oliverchang is it possible to reopen this PR ?

@catenacyber catenacyber mentioned this pull request Dec 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.