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

nvalid IPv6 URL #6772

Open
x1280 opened this issue May 31, 2022 · 27 comments
Open

nvalid IPv6 URL #6772

x1280 opened this issue May 31, 2022 · 27 comments
Labels
bug invalid This doesn't seem right

Comments

@x1280
Copy link

x1280 commented May 31, 2022

No description provided.

@x1280 x1280 added the bug label May 31, 2022
@Dreamsorcerer
Copy link
Contributor

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
Copy link
Member

webknjaz commented Jun 26, 2022

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
Copy link

@Rongronggg9
Copy link

Rongronggg9 commented Jun 27, 2022

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
Copy link
Member

webknjaz commented Jun 27, 2022

$ 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
Copy link
Member

I can't say anything but lol

Exactly what I was thinking. Note that there's some discussions along the same lines @ #6801.

@woodruffw
Copy link

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
Copy link

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 google/oss-fuzz#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 #5320

Again, many apologies that this false security filing happened.

@tooptoop4

This comment was marked as spam.

@webknjaz
Copy link
Member

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: #5320 (review). Running the fuzzers could be controlled by pytest markers, for example.

@linkwithkk
Copy link

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
Copy link

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 #6772 (comment) I think this is not the view of the maintainers.

@Dreamsorcerer
Copy link
Contributor

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 (#6772 (comment)), 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
Copy link

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
Copy link

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
Copy link

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
Copy link
Member

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 🤷‍♂️.

@webknjaz
Copy link
Member

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...

@webknjaz webknjaz added the invalid This doesn't seem right label Aug 22, 2022
@Rongronggg9
Copy link

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
Copy link

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
Copy link
Member

I tried reaching out to MITRE themselves

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

@woodruffw
Copy link

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
Copy link
Member

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

Yeah, it got distributed across two issues... #6801 (comment)

@nrpt-m
Copy link

nrpt-m commented Feb 9, 2023

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

@webknjaz
Copy link
Member

webknjaz commented Feb 9, 2023

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.

@tharradine
Copy link

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
Copy link
Member

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests