aio-libs / aiohttp

Asynchronous HTTP client/server framework for asyncio and Python
https://docs.aiohttp.org
Other
15.18k stars 2.03k forks source link

<This issue is referenced from a **SPAM**/nonsense CVE with no explanation and no good reason. Ignore it as there is no actual vulnerability here> #6772

Closed x1280 closed 1 year ago

Dreamsorcerer commented 2 years ago

The code appears to be catching a HttpProcessingError exception: https://github.com/google/oss-fuzz/blob/master/projects/aiohttp/fuzz_http_parser.py#L42

So, maybe our parser should consider catching and rethrowing this exception.

webknjaz commented 2 years ago

[Uploading crash.zip…]()

This is a link to the current issue, not an actual attachment. Do you have any proof that this is actually a problem?

Denial of service

Why do you think it's expected?

ValueError: Invalid IPv6 URL
Traceback (most recent call last):
  File "fuzz_http_parser.py", line 32, in TestOneInput
  File "aiohttp/_http_parser.pyx", line 551, in aiohttp._http_parser.HttpParser.feed_data
  File "aiohttp/_http_parser.pyx", line 701, in aiohttp._http_parser.cb_on_header_field
  File "aiohttp/_http_parser.pyx", line 627, in aiohttp._http_parser.HttpRequestParser._on_status_complete
  File "yarl/_url.py", line 151, in __new__
  File "urllib/parse.py", line 464, in urlsplit

This only demonstrates a traceback coming from stdlib (urllib) and yarl. So what's an actual complaint in the aiohttp-land?

giorgiop commented 2 years ago

FYI https://security.snyk.io/vuln/SNYK-PYTHON-AIOHTTP-2934978

Rongronggg9 commented 2 years ago

Now everyone gets informed that there is a vulnerability "waiting for a PoC", but everyone just can't see any evidence indicating that aiohttp is vulnerable.

CVE-2022-33124

DISPUTED AIOHTTP 3.8.1 can report a "ValueError: Invalid IPv6 URL" outcome, which can lead to a Denial of Service (DoS). NOTE: multiple third parties dispute this issue because there is no example of a context in which denial of service would occur, and many common contexts have exception handing in the calling application.

Without any PoC, if only raising an exception can be considered to be a DoS vulnerability instead of a bug or expected behavior, well, should CVE-ID be 64bit long?

aiohttp v3.8.1 was discovered to contain an invalid IPv6 URL which can lead to a Denial of Service (DoS).

I can't say anything but lol


So, please show your "example of a context in which denial of service would occur". Don't make unnecessary misunderstandings happen (already happened if it does not be a vulnerability since a CVE-ID has been registered).

webknjaz commented 2 years ago
$ python -m pip show aiohttp
latest

I also wonder what the latest means in this context. Since it's clearly not an output that the above command produces but something user-invented, it's hard to judge what it refers to. Is it the latest version published to PyPI? Or is it something built from source. (I think it's the latter because of how the paths show up in the traceback). If it's been built from the source, there's more questions to answer on how it's been built and whether it was built from the HEAD of this repo's default branch or maybe from a minor version branch HEAD. Without this, it's hard to guess what exactly this issue is implying even.

webknjaz commented 2 years ago

I can't say anything but lol

Exactly what I was thinking. Note that there's some discussions along the same lines @ https://github.com/aio-libs/aiohttp/issues/6801.

woodruffw commented 2 years ago

For pip-audit users, you can temporarily ignore this disputed vulnerability using the --ignore-vuln flag:

$ pip-audit --ignore-vuln GHSA-rwqr-c348-m5wr
DavidKorczynski commented 2 years ago

I'm the author of the fuzzers in the OSS-Fuzz repo (https://github.com/google/oss-fuzz/tree/master/projects/aiohttp). Sorry to see this mess -- the CVE filing or security filing does not come from OSS-Fuzz and am not sure who files them. This has previously happened, e.g. see this OSS-Fuzz issue which deals with unknown CVE filings https://github.com/google/oss-fuzz/issues/7146.

In this context, I would be happy to develop the fuzzing suite into aiohttp and submit it all upstream so you can review and ensure it matches your threat model, i.e. I would be happy to revive this PR https://github.com/aio-libs/aiohttp/pull/5320

Again, many apologies that this false security filing happened.

tooptoop4 commented 2 years ago

yo aqua/cve: update ur db and processes, dis issue got me f#*d up

webknjaz commented 2 years ago

I would be happy to revive this PR #5320

Yeah, that could be useful but in order to proceed, it really has to be integrated into the pytest setup we've got first, as I mentioned earlier: https://github.com/aio-libs/aiohttp/pull/5320#pullrequestreview-546419621. Running the fuzzers could be controlled by pytest markers, for example.

linkwithkk commented 2 years ago

Any idea or timeline when the above PR would be done ? This is still up there in nvd so most code scanning tools are still reporting it as vulnerable. We are now left with very few options as this has been around for 2 months and since it's still up there in nvd, we are being forced to find replacements and get rid of libs dependent on this.

DavidKorczynski commented 2 years ago

Any idea or timeline when the above PR would be done ?

Are you referring to the PR I discuss? If so, reviving that PR will have no impact on the code of aiohttp itself and won't impact what's reported here.

Following along this issue, the maintainers of aiohttp do not consider this a vulnerability, i.e. the argument is the scanning tools are reporting something wrong. I cannot comment too much on whether the scanning tools have responsibility to verify in detail the info they present, but there seems to be a conflict of some sort at least.

I have not gone into details with the threat model myself so I cannot comment on that. So, I will assume there is no security issue present here. Furthermore, the missing catching of exceptions of urllib happens across many projects (despite being specified as being thrown https://docs.python.org/3/library/urllib.parse.html#url-parsing) and I'm not entirely what to think of that. In a sense I would assume if the API specifies exceptions as being thrown this should be handled by the library, however, based on the response here https://github.com/aio-libs/aiohttp/issues/6772#issuecomment-1166617766 I think this is not the view of the maintainers.

Dreamsorcerer commented 2 years ago

Following along this issue, the maintainers of aiohttp do not consider this a vulnerability, i.e. the argument is the scanning tools are reporting something wrong.

Furthermore, you'd expect any security vulnerability with a CVE to have already have had someone contact us privately and provide evidence of a vulnerability and give us time to deal with it. To this date, nobody has reported any security issue to us this year. Only users asking us what this CVE is about (who heard about it even before us).

Furthermore, the missing catching of exceptions of urllib happens across many projects (despite being specified as being thrown https://docs.python.org/3/library/urllib.parse.html#url-parsing) and I'm not entirely what to think of that.

If arbitrary unhandled exceptions are a security vulnerability, then aiohttp and every other framework has an infinite number of vulnerabilities. As I mentioned at the beginning, we could transform the exception (https://github.com/aio-libs/aiohttp/issues/6772#issuecomment-1142424959), to make error handling easier. But, there just doesn't appear to be any evidence that this creates a DoS attack, it's just a usability issue.

linkwithkk commented 2 years ago

Thanks for the help and replies. I'm aware of the fact that this isn't actually a security vulnerabilities however the CVE is still active for version 3.8.1 (https://nvd.nist.gov/vuln/detail/CVE-2022-33124) Not sure who or what has created this mess but because of this, code scanning tools which refer to the above are still reporting v3.8.1 as having security vulnerabilities. I was of the impression that with a new release the above (pseudo?)CVE will not be applicable.

woodruffw commented 2 years ago

My understanding is that it was a process breakdown: someone directly reported a "crash" they found with OSS-fuzz and got it accepted by one of the CVE assigning authorities, rather than letting Google triage it or reporting it upstream (where the maintainers would have rejected it).

NVD currently shows the CVE as disputed, which your scanning tool should be able to filter on. I believe people are still trying to contact NIST/NVD to get them to yank it entirely, since it's completely invalid.

Rongronggg9 commented 2 years ago

I was of the impression that with a new release the above (pseudo?)CVE will not be applicable.

I am not a maintainer of aiohttp. I doubt if aiohttp did do a release to "fix" the CVE, it might be much more complex to make the false-positive CVE be revoked.

webknjaz commented 2 years ago

code scanning tools which refer to the above are still reporting v3.8.1 as having security vulnerabilities.

There are two problems here: 1) The code scanning tools should recognize disputed and withdrawn statuses. It's on them to support this use-case, and maybe allow the end-users to mark warnings as nonsense. 2) Using fake CVEs to justify bullying project maintainers into cutting a new release out of annoyance would set a bad precedent when things aren't great already. If somebody makes a PR to improve the exception handling, that'll be reviewed, of course. And maybe it'd correlate with our ability to release, but there's no direct connection between these, and making dummy releases to shut up tools that aren't related to us in any way seems suboptimal.

On the note of making a new release, the current blocker is getting the CI green under Python 3.11 which is in progress. Once that is solved, we'll look into including some unmerged PRs if that'll make sense and make a release. Not sooner.

I believe people are still trying to contact NIST/NVD to get them to yank it entirely, since it's completely invalid.

Not me, I gave up on that after having filled out the forms per their processes. They are ghosting me (and I assume others too?) + there's no alternative contact, so I don't see what else I can do regarding fighting this CVE-spam :man_shrugging:.

webknjaz commented 2 years ago

Interestingly, that "CVE" links to this issue with the "Exploit" and "Third Party Advisory" labels. I wonder if they'll (automatically) notice that it's invalid if I apply an invalid label here...

Rongronggg9 commented 2 years ago

They are ghosting me (and I assume others too?) + there's no alternative contact

I wonder if GitHub will come to help, at least it is a CNA (though the CVE-ID was not assigned by it).

A joke No, the issue is not invalid, never. It is **nvalid**!
woodruffw commented 2 years ago

Not me, I gave up on that after having filled out the forms per their processes. They are ghosting me (and I assume others too?) + there's no alternative contact, so I don't see what else I can do regarding fighting this CVE-spam 🤷‍♂️.

I probably won't have any more luck, but I tried reaching out to MITRE themselves (rather than NIST/NVD) as well.

webknjaz commented 2 years ago

I tried reaching out to MITRE themselves

I believe that's exactly what me and the others did too.

woodruffw commented 2 years ago

I believe that's exactly what me and the others did too.

Ah, dang. In that case I'm probably retracing your exact steps 😅; sorry for the confusion.

webknjaz commented 2 years ago

Ah, dang. In that case I'm probably retracing your exact steps :sweat_smile:; sorry for the confusion.

Yeah, it got distributed across two issues... https://github.com/aio-libs/aiohttp/issues/6801#issuecomment-1167371253

nrpt-m commented 1 year ago

@webknjaz If CVE-2022-33124 has been confirmed as invalid or disputed then why this issue is still open ?

webknjaz commented 1 year ago

This issue is probably a legit annoyance but we don't know why that "CVE" is pointing at it. There's no explanation of what that link means.

ghost commented 1 year ago

Annoyingly, this "vulnerability" has just been incorporated into pyupio/safety-db, so it's now going to cause issues for anyone using the safety tool for dependency scanning. I feel sorry for the awesome aiohttp maintainers in case more users start raising this again because another vulnerability database contains a false entry :(

Just posting this message as an FYI for anyone using safety and stumbling upon this thread whilst researching the vulnerability. I've raised pyupio/safety-db#2363 on that repository to ask why it was added after it has obviously been withdrawn.

Regards, Toby

webknjaz commented 1 year ago

Thank you, Toby! I'm on the PyCon Sprints so if anybody's around and want to hack on this, trying to figure out what the TS wanted together, hit me up!

DoubleHelixX commented 1 year ago

Any updates on this? Very unfortunate. Hope this gets resolved soon!

webknjaz commented 1 year ago

Feel free to submit a PR!

Julian commented 1 year ago

I think this "vulnerability" has now just made its way into pypa/advisory-database, or at least I am now getting pip-audit errors about it where I wasn't a few hours ago. Assuming it's not something I've missed, there may be yet another wave of people hitting this issue. (EDIT: I see of course it's not new in the advisory DB, but I again have not seen it before until a few minutes ago, so something seems likely to have changed).

If so, in case it helps anyone finding this, my fix given the above is simply pip-audit --ignore-vuln PYSEC-2022-43059.

woodruffw commented 1 year ago

Hmm, maybe it wasn't given a PYSEC ID before? Looking into it now.

woodruffw commented 1 year ago

Ah yep, I knew I remembered this: https://github.com/pypa/advisory-database/issues/83

webknjaz commented 1 year ago

Oh, again? Urgh.. I suppose MITRE is ghosting all of us :man_shrugging:

woodruffw commented 1 year ago

Yep :upside_down_face:

From a quick look, I think what happened here is that the GHSA advisory was correctly withdrawn, but (as we all know) the CVE never was. PYSEC didn't actually have an "active" advisory open for this until the CVE one got merged in somehow, causing it to ignore the fact that it's already been withdrawn.

woodruffw commented 1 year ago

This is now withdrawn from both PYSEC and GHSA, as of https://github.com/pypa/advisory-database/pull/169.

There's been no update from MITRE, so I think we can safely assume that they're not going to perform any diligence here.

@webknjaz Give the above, I think this is safe to close. WDYT?

webknjaz commented 1 year ago

@woodruffw fair enough. Closing… Thanks everyone involved/annoyed :)

P.S. The latest update on the CVE page, down in the history:

CPE Deprecation Remap by NIST 11/22/2023 12:09:17 PM Action Type Old Value New Value
Changed CPE Configuration OR
*cpe:2.3:a:aiohttp_project:aiohttp:3.8.1:*:*:*:*:*:*:*
OR
*cpe:2.3:a:aiohttp:aiohttp:3.8.1:*:*:*:*:*:*:*