#10826: [Security] Safe Haskell can be bypassed via annotations -------------------------------------+------------------------------------- Reporter: spinda | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: GHC accepts | Unknown/Multiple invalid program | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Revisions: -------------------------------------+------------------------------------- Description changed by spinda: Old description:

{{{ module Test (hook) where

import System.Process import System.IO.Unsafe

{-# ANN hook (unsafePerformIO (putStrLn "Woops.")) #-} hook = undefined }}}

{{{ ➜ Test ghc -fpackage-trust -XSafe Test_simple.hs [1 of 1] Compiling Test_simple ( Test_simple.hs, Test_simple.o ) [flags changed] Woops.

Test_simple.hs:4:1: System.IO.Unsafe: Can't be safely imported! The module itself isn't safe. }}}

GHC ultimately rejects the program due to the {{{System.IO.Unsafe}}} import, but this check doesn't occur until GHC has compiled and run the annotation expression, allowing arbitrary IO operations via {{{unsafePerformIO}}}.

The solution is probably to move the import check from the end of renaming/typechecking to the start.

New description: {{{ module Test (hook) where import System.IO.Unsafe {-# ANN hook (unsafePerformIO (putStrLn "Woops.")) #-} hook = undefined }}} {{{ ➜ Test ghc -fpackage-trust -XSafe Test_simple.hs [1 of 1] Compiling Test_simple ( Test_simple.hs, Test_simple.o ) [flags changed] Woops. Test_simple.hs:4:1: System.IO.Unsafe: Can't be safely imported! The module itself isn't safe. }}} GHC ultimately rejects the program due to the {{{System.IO.Unsafe}}} import, but this check doesn't occur until GHC has compiled and run the annotation expression, allowing arbitrary IO operations via {{{unsafePerformIO}}}. The solution is probably to move the import check from the end of renaming/typechecking to the start. -- -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/10826#comment:1 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#10826: [Security] Safe Haskell can be bypassed via annotations -------------------------------------+------------------------------------- Reporter: spinda | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: GHC accepts | Unknown/Multiple invalid program | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Revisions: -------------------------------------+------------------------------------- Comment (by spinda): I should note that checking imports after renaming/typechecking, instead of before, also opens up nasty possibilities with QuasiQuoters, since Safe Haskell leaves them enabled (despite disabling the rest of Template Haskell). I have a more involved proof of concept that uses these two in conjunction to both execute arbitrary IO operations and delve into the GHC internals through a QuasiQuoter to mark arbitrary modules as safe. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/10826#comment:3 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#10826: [Security] Safe Haskell can be bypassed via annotations -------------------------------------+------------------------------------- Reporter: spinda | Owner: Type: bug | Status: new Priority: highest | Milestone: 7.10.3 Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: GHC accepts | Unknown/Multiple invalid program | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Revisions: -------------------------------------+------------------------------------- Comment (by kanetw): I don't think it's possible to run the import check before typechecking/renaming as it requires a TcGblEnv (see checkSafeImports). You could run rnImports/maybe tcRnImports first, then check safety based on those imports, then do the rest. But I'm not sure whether that'll be sufficient for Safe Haskell. I'll take a closer look tomorrow. I have a patch that just plain disables annotations under Safe Haskell; if that's acceptable (as a workaround at least) I can post it on Phab. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/10826#comment:5 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#10826: [Security] Safe Haskell can be bypassed via annotations -------------------------------------+------------------------------------- Reporter: spinda | Owner: Type: bug | Status: new Priority: highest | Milestone: 7.10.3 Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: GHC accepts | Unknown/Multiple invalid program | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Revisions: -------------------------------------+------------------------------------- Comment (by kanetw): Not as far as I know. They're only accessible via the GHC API or TH and that's unsafe Haskell, never mind that you have to explicitly unhide `ghc` for the first way. .hi files do have annotations stored, so you could read them with an external tool. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/10826#comment:7 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#10826: [Security] Safe Haskell can be bypassed via annotations -------------------------------------+------------------------------------- Reporter: spinda | Owner: Type: bug | Status: new Priority: highest | Milestone: 7.10.3 Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: GHC accepts | Unknown/Multiple invalid program | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Revisions: -------------------------------------+------------------------------------- Comment (by goldfire): Sounds to me like we should just disable annotations entirely in Safe Haskell. I'm sure someone can be clever enough to sort out the monads, but no one is requesting this feature, to my knowledge. And it would be peculiar (but certainly conceivable) to use Safe Haskell and then inspect .hi files manually (or through `--show-iface`). So, unless there are objections: disable annotations in Safe Haskell. Do please add a note in the release notes about this. Given that it's a safety issue, I think it's reasonable to mark this "merge" so that the fix goes into 7.10.3. But others may differ here, as the change could kill existing non-malicious code. Also, in the error message that happens when a user tries an annotation in Safe Haskell, I think it would be best to include a link to this ticket, so that users who ''do'' want the feature have a nice place to make themselves known. Many thanks! -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/10826#comment:9 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#10826: [Security] Safe Haskell can be bypassed via annotations -------------------------------------+------------------------------------- Reporter: spinda | Owner: kanetw Type: bug | Status: patch Priority: highest | Milestone: 7.10.3 Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: GHC accepts | Unknown/Multiple invalid program | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Revisions: phab:D1226 -------------------------------------+------------------------------------- Changes (by kanetw): * status: new => patch * differential: => phab:D1226 -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/10826#comment:11 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#10826: [Security] Safe Haskell can be bypassed via annotations -------------------------------------+------------------------------------- Reporter: spinda | Owner: kanetw Type: bug | Status: merge Priority: highest | Milestone: 7.10.3 Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: GHC accepts | Unknown/Multiple invalid program | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Revisions: phab:D1226 -------------------------------------+------------------------------------- Changes (by thomie): * status: patch => merge -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/10826#comment:13 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler