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

GHC 9.0.1 Natural conversion bug #210

Open
hsyl20 opened this issue Jun 17, 2024 · 7 comments · May be fixed by #214
Open

GHC 9.0.1 Natural conversion bug #210

hsyl20 opened this issue Jun 17, 2024 · 7 comments · May be fixed by #214

Comments

@hsyl20
Copy link

hsyl20 commented Jun 17, 2024

Mandatory information:

  • Package : ghc
  • cvss: ?
  • affected versions: 9.0.1

Optional:

  • cve: ?
  • keywords: ?
  • aliases: ?
  • related: ?
  • affected OSes: all
  • affected architecture: all
  • declarations: ?
  • Long description: ?

GHC 9.0.1's optimization of Natural numbers can't be trusted because of a bogus rewrite rule. See https://gitlab.haskell.org/ghc/ghc/-/issues/19345, https://gitlab.haskell.org/ghc/ghc/-/issues/20066, https://gitlab.haskell.org/ghc/ghc/-/merge_requests/4980, and https://gitlab.haskell.org/ghc/ghc/-/merge_requests/6109.

Not exactly sure how it might be used for an attack but we quickly discussed this issue at the HF workshop and concluded it was worth reporting anyway.

@TristanCacqueray
Copy link
Collaborator

It seems like this bug fall into CWE-192: Integer Coercion Error. I guess it deserves an advisory because of the resulting availability and data integrity issues.

@TristanCacqueray
Copy link
Collaborator

@frasertweedale I guess this is not hackage advisory, I remember we talked about using advisories/ghc for this kind of issue, but it seem like we still need to add support for this in hsec tool right?

@hsyl20 Looking for references, I also found https://gitlab.haskell.org/ghc/ghc/-/issues/22282 (GHC 9.4.2 regresses being able to do math on aarch64), should we issue an advisory for this as well?

@frasertweedale
Copy link
Collaborator

@hsyl20 thank you for the report.

@TristanCacqueray if we file it, I would record it against base in our hackage namespace, as we did for the readFloat issue in HSEC-2023-0007.

But I am not sure we should file it - unlike readFloat which presented a clear DoS risk. Instead, I propose that if some package is found to have a vulnerability arising from this behaviour, the advisory would be issued against that package. We would re-evaluate at that time whether an advisory should also be issued again base.

That's my starting position. SRT will discuss and decide.

@TristanCacqueray
Copy link
Collaborator

I remember reading about an actual DoS but I can't find the reference anymore: the user was computing a checksum for a large file. So I think we should file this. Looking at the fix, it seems to be only touching base so I guess the affected version should be 4.15.0.0, and this was fixed in 4.15.1.0

@hsyl20
Copy link
Author

hsyl20 commented Jun 21, 2024

@hsyl20 Looking for references, I also found https://gitlab.haskell.org/ghc/ghc/-/issues/22282 (GHC 9.4.2 regresses being able to do math on aarch64), should we issue an advisory for this as well?

If we follow this trend, it might be faster to indicate which GHC versions don't have math/numeric bugs. E.g. on PowerPC all versions since 9.2 are subject to https://gitlab.haskell.org/ghc/ghc/-/issues/23034 (bug with signed numbers and FFI calls, cf https://gitlab.haskell.org/ghc/ghc/-/merge_requests/12885).

More seriously, yes we should probably add an advisory for 9.4.2 on aarch64 because of this bug.

@hasufell
Copy link
Member

GHC 9.0.1's optimization of Natural numbers can't be trusted because of a bogus rewrite rule.

Is 9.4.8 affected by this?

@hsyl20
Copy link
Author

hsyl20 commented Jun 26, 2024

Is 9.4.8 affected by this?

I don't think so. It has been fixed pretty quickly in 9.0.2 and master at the time. Maybe you're thinking about another issue?

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 a pull request may close this issue.

4 participants