close off newline injection attacks against compute special remote protocol

diff --git a/Remote/Compute.hs b/Remote/Compute.hs
index 7ed6040cebaddb4e27d1d007f9de8177b38203c1..0b27d135ba4f548948a6bb16c4906d91e9189980 100644 (file)
--- a/Remote/Compute.hs
+++ b/Remote/Compute.hs
@@ -56,6 +56,7 @@ import Utility.CopyFile
 import Types.Key
 import Backend
 import qualified Git
+import qualified Utility.OsString as OS
 import qualified Utility.FileIO as F
 import qualified Utility.RawFilePath as R
 import qualified Utility.SimpleProtocol as Proto
@@ -271,7 +272,9 @@ formatComputeState' mk st = renderQuery False $ concat
 parseComputeState :: Key -> B.ByteString -> Maybe ComputeState
 parseComputeState k b =
        let st = go emptycomputestate (parseQuery b)
-       in if st == emptycomputestate then Nothing else Just st
+       in if st == emptycomputestate || illegalComputeState st
+               then Nothing
+               else Just st
   where
        emptycomputestate = ComputeState 
                { computeParams = mempty
@@ -317,6 +320,20 @@ parseComputeState k b =
                        _ -> Nothing
                in go c' rest
 
+{- This is used to avoid ComputeStates that should never happen,
+ - but which could be injected into a repository by an attacker. -}
+illegalComputeState :: ComputeState -> Bool
+illegalComputeState st
+       -- The protocol is line-based, so filenames used in it cannot
+       -- contain newlines.
+       | any containsnewline (M.keys (computeInputs st)) = True
+       | any containsnewline (M.keys (computeOutputs st)) = True
+       -- Just in case.
+       | containsnewline (computeSubdir st) = True
+       | otherwise = False
+  where
+       containsnewline p = unsafeFromChar '\n' `OS.elem` p
+
 {- A compute: url for a given output file of a computation. -}
 computeStateUrl :: Remote -> ComputeState -> OsPath -> URLString
 computeStateUrl r st p = 

diff --git a/doc/todo/compute_special_remote_remaining_todos.mdwn b/doc/todo/compute_special_remote_remaining_todos.mdwn
index fab644f0e448b96f309c06edca1ac4e901b289e3..bba17b23006d39d0c6aa596e2b1fbbaa0ec59f76 100644 (file)
--- a/doc/todo/compute_special_remote_remaining_todos.mdwn
+++ b/doc/todo/compute_special_remote_remaining_todos.mdwn
@@ -1,10 +1,6 @@
 This is the remainder of my todo list while I was building the
 compute special remote. --[[Joey]]
 
-* prohibit using compute states where an input or output filename contains
-  a newline. The protocol doesn't allow this to happen usually, but an 
-  attacker might try it in order to scramble the protocol.
-
 * git-annex responds to each INPUT immediately, and flushes stdout.
   This could cause problems if the program is sending several INPUT
   first, before reading responses, as is documented it should do to allow