Skip to content
  • Jeff King's avatar
    submodule-config: verify submodule names as paths · 0383bbb9
    Jeff King authored
    
    
    Submodule "names" come from the untrusted .gitmodules file,
    but we blindly append them to $GIT_DIR/modules to create our
    on-disk repo paths. This means you can do bad things by
    putting "../" into the name (among other things).
    
    Let's sanity-check these names to avoid building a path that
    can be exploited. There are two main decisions:
    
      1. What should the allowed syntax be?
    
         It's tempting to reuse verify_path(), since submodule
         names typically come from in-repo paths. But there are
         two reasons not to:
    
           a. It's technically more strict than what we need, as
              we really care only about breaking out of the
              $GIT_DIR/modules/ hierarchy.  E.g., having a
              submodule named "foo/.git" isn't actually
              dangerous, and it's possible that somebody has
              manually given such a funny name.
    
           b. Since we'll eventually use this checking logic in
              fsck to prevent downstream repositories, it should
              be consistent across platforms. Because
              verify_path() relies on is_dir_sep(), it wouldn't
              block "foo\..\bar" on a non-Windows machine.
    
      2. Where should we enforce it? These days most of the
         .gitmodules reads go through submodule-config.c, so
         I've put it there in the reading step. That should
         cover all of the C code.
    
         We also construct the name for "git submodule add"
         inside the git-submodule.sh script. This is probably
         not a big deal for security since the name is coming
         from the user anyway, but it would be polite to remind
         them if the name they pick is invalid (and we need to
         expose the name-checker to the shell anyway for our
         test scripts).
    
         This patch issues a warning when reading .gitmodules
         and just ignores the related config entry completely.
         This will generally end up producing a sensible error,
         as it works the same as a .gitmodules file which is
         missing a submodule entry (so "submodule update" will
         barf, but "git clone --recurse-submodules" will print
         an error but not abort the clone.
    
         There is one minor oddity, which is that we print the
         warning once per malformed config key (since that's how
         the config subsystem gives us the entries). So in the
         new test, for example, the user would see three
         warnings. That's OK, since the intent is that this case
         should never come up outside of malicious repositories
         (and then it might even benefit the user to see the
         message multiple times).
    
    Credit for finding this vulnerability and the proof of
    concept from which the test script was adapted goes to
    Etienne Stalmans.
    
    Signed-off-by: default avatarJeff King <peff@peff.net>
    0383bbb9