fix Arbitrary AssociatedFile

Empty filenames were already filtered out as not allowed. But before
the change to ByteString, a NUL could appear in an Arbitrary String,
and so Arbitrary AssociatedFile sometimes generated illegal filenames,
as NUL never appears in a filename. The change to ByteString meant the
String was run through toRawFilePath, which assumes a filename never
contains a NUL. That truncated the String at the NUL, which could
result in an AssociatedFile being generated with an empty filename.

The filtering of NUL added here is not really necessary, because
of the truncation, but it makes explicit that NUL is not allowed.
The real fix is that the suchThat now applies to the final
AssociatedFile, so will catch any empty ones however generated.

This raises the more general question of whether toRawFilePath might
truncate other strings that later get used as filenames. I think new
bugs probably won't be introduced by that. Before, a FilePath that got
read from somewhere (eg an attacker) and contained a NUL would perhaps
be printed out by git-annex, including the NUL, or written to disk
inside a file, or what have you. But as soon as that FilePath gets
passed to any IO action that treats it as a filename, it gets truncated
after the NUL. Eg, writeFile "foo\NULbar" "bar" writes to file "foo".
Now toRawFilePath will make the truncation happen earler, but at most
this will affect what gets printed out or is written to disk inside a
file; actually using the RawFilePath as a filename will not change from
using the FilePath as a filename.
This commit is contained in:
Joey Hess 2019-12-06 12:44:18 -04:00
parent faf5415163
commit 3ece4758c6
No known key found for this signature in database
GPG key ID: DB12DB0FF05F8F38

9
Key.hs
View file

@ -78,10 +78,13 @@ instance Arbitrary KeyData where
<*> ((abs <$>) <$> arbitrary) -- chunksize cannot be negative <*> ((abs <$>) <$> arbitrary) -- chunksize cannot be negative
<*> ((succ . abs <$>) <$> arbitrary) -- chunknum cannot be 0 or negative <*> ((succ . abs <$>) <$> arbitrary) -- chunknum cannot be 0 or negative
-- AssociatedFile cannot be empty (but can be Nothing) -- AssociatedFile cannot be empty, and cannot contain a NUL
-- (but can be Nothing)
instance Arbitrary AssociatedFile where instance Arbitrary AssociatedFile where
arbitrary = AssociatedFile . fmap toRawFilePath arbitrary = (AssociatedFile . fmap mk <$> arbitrary)
<$> arbitrary `suchThat` (/= Just "") `suchThat` (/= AssociatedFile (Just S.empty))
where
mk = toRawFilePath . filter (/= '\NUL')
instance Arbitrary Key where instance Arbitrary Key where
arbitrary = mkKey . const <$> arbitrary arbitrary = mkKey . const <$> arbitrary