Am 10.04.2017 um 18:57 schrieb Jeff King:
If there are security bugs where a malicious input can cause us
to do something bad, that's something to care about. But that's very
different than asking "do these tests run to completion with a funny
input".
If the tests do not complete, git is doing something unexpected.
I very much disagree with that. Git's test operate under a set of
assumptions, and if you violate those assumptions, then the failures are
not meaningful.
In that case the tests do not validate that git can properly work with
special characters.
That's a pretty big coverage gap.
Take alternates, for instance. The on-disk storage format cannot
represent paths with newlines in them. If a test does:
git clone -s "$(pwd)" parent.git child &&
test -d child
then that test is going to fail if the test directory has a newline in
it. But that doesn't tell us anything meaningful. Maybe there is a bug
and maybe there isn't, but we cannot know because the thing being tested
cannot possibly work under the test environment given.
Sure. Not all tests are meaningful in all environments.
That doesn't mean that the tests are generally meaningless.
Also, we're talking about two pretty different things here: gits
interaction with the file system, and git's interaction with whatever
shell its scripts are using.
In an ideal world, these two aspects would be orthogonal and could be
tested independently of each other. Since in practice we do have
correlations and dependencies, this isn't always possible, but it's what
we should aim for.
You can rewrite all the tests to say "skip this test if there's a
newline in the test directory". But to what end? It's work to write and
to maintain, and we haven't gained new information.
Not on that "alternates" thing (whatever that is), but we have a test
that will work and provide information on systems that do allow newlines.
That in itself is not a security hole, but there's a pretty good chance that
at least one of these ~230 unexpected things can be turned into one, given
enough time and motivation. The risk multiplies as this is shell scripting,
where the path from "string is misinterpreted" to "string is run as a
command" is considerably shorter than in other languages.
Sure, and I'd encourage people who are interested to dig through the
results and see if they can find a real problem. I looked at several and
didn't find anything that wasn't an example of the "test assumptions"
thing above.
Don't assume that there's no risk just because you didn't find anything.
Also, git might not be the actual hole, but other software that relies
on git not doing anything awkward might fail that assumption and expose
the actual breach of security.
You could argue that it's not a problem in git, but it is. Unless and
until the git docs clearly state that things may break if "funny
characters" are being used, and where.
But again, I'm happy to be proven wrong.
With security, you need to be confident about the absence of /any/ type
of hole, not the absence of a /specific/ type of hole such as a shell
injection.
So the way forward isn't proving you wrong by providing a specific
exploit, it's making sure that no exploit with URLs and file names can
possibly exist.
Now if I'm reading things like "heuristics" and "git uses URL-specific
code even after it has determined it's not a URL"... well, that's the
exact opposite of reassuring messages.
> I just don't think
plastering control characters into the test directory names all the time
is a good way of finding those problems (and doesn't balance out the
cost).
Fair enough.