1. 27 Sep, 2018 6 commits
    • Junio C Hamano's avatar
      Git 2.15.3 · 924c623e
      Junio C Hamano authored
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      924c623e
    • Junio C Hamano's avatar
      Sync with Git 2.14.4 · 902df9f5
      Junio C Hamano authored
      * maint-2.14:
        Git 2.14.5
        submodule-config: ban submodule paths that start with a dash
        submodule-config: ban submodule urls that start with dash
        submodule--helper: use "--" to signal end of clone options
      902df9f5
    • Junio C Hamano's avatar
      Git 2.14.5 · d0832b28
      Junio C Hamano authored
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      d0832b28
    • Jeff King's avatar
      submodule-config: ban submodule paths that start with a dash · 273c6149
      Jeff King authored
      We recently banned submodule urls that look like
      command-line options. This is the matching change to ban
      leading-dash paths.
      
      As with the urls, this should not break any use cases that
      currently work. Even with our "--" separator passed to
      git-clone, git-submodule.sh gets confused. Without the code
      portion of this patch, the clone of "-sub" added in t7417
      would yield results like:
      
          /path/to/git-submodule: 410: cd: Illegal option -s
          /path/to/git-submodule: 417: cd: Illegal option -s
          /path/to/git-submodule: 410: cd: Illegal option -s
          /path/to/git-submodule: 417: cd: Illegal option -s
          Fetched in submodule path '-sub', but it did not contain b56243f8f4eb91b2f1f8109452e659f14dd3fbe4. Direct fetching of that commit failed.
      
      Moreover, naively adding such a submodule doesn't work:
      
        $ git submodule add $url -sub
        The following path is ignored by one of your .gitignore files:
        -sub
      
      even though there is no such ignore pattern (the test script
      hacks around this with a well-placed "git mv").
      
      Unlike leading-dash urls, though, it's possible that such a
      path _could_ be useful if we eventually made it work. So
      this commit should be seen not as recommending a particular
      policy, but rather temporarily closing off a broken and
      possibly dangerous code-path. We may revisit this decision
      later.
      
      There are two minor differences to the tests in t7416 (that
      covered urls):
      
        1. We don't have a "./-sub" escape hatch to make this
           work, since the submodule code expects to be able to
           match canonical index names to the path field (so you
           are free to add submodule config with that path, but we
           would never actually use it, since an index entry would
           never start with "./").
      
        2. After this patch, cloning actually succeeds. Since we
           ignore the submodule.*.path value, we fail to find a
           config stanza for our submodule at all, and simply
           treat it as inactive. We still check for the "ignoring"
           message.
      Signed-off-by: default avatarJeff King <peff@peff.net>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      273c6149
    • Jeff King's avatar
      submodule-config: ban submodule urls that start with dash · f6adec4e
      Jeff King authored
      The previous commit taught the submodule code to invoke our
      "git clone $url $path" with a "--" separator so that we
      aren't confused by urls or paths that start with dashes.
      
      However, that's just one code path. It's not clear if there
      are others, and it would be an easy mistake to add one in
      the future. Moreover, even with the fix in the previous
      commit, it's quite hard to actually do anything useful with
      such an entry. Any url starting with a dash must fall into
      one of three categories:
      
       - it's meant as a file url, like "-path". But then any
         clone is not going to have the matching path, since it's
         by definition relative inside the newly created clone. If
         you spell it as "./-path", the submodule code sees the
         "/" and translates this to an absolute path, so it at
         least works (assuming the receiver has the same
         filesystem layout as you). But that trick does not apply
         for a bare "-path".
      
       - it's meant as an ssh url, like "-host:path". But this
         already doesn't work, as we explicitly disallow ssh
         hostnames that begin with a dash (to avoid option
         injection against ssh).
      
       - it's a remote-helper scheme, like "-scheme::data". This
         _could_ work if the receiver bends over backwards and
         creates a funny-named helper like "git-remote--scheme".
         But normally there would not be any helper that matches.
      
      Since such a url does not work today and is not likely to do
      anything useful in the future, let's simply disallow them
      entirely. That protects the existing "git clone" path (in a
      belt-and-suspenders way), along with any others that might
      exist.
      
      Our tests cover two cases:
      
        1. A file url with "./" continues to work, showing that
           there's an escape hatch for people with truly silly
           repo names.
      
        2. A url starting with "-" is rejected.
      
      Note that we expect case (2) to fail, but it would have done
      so even without this commit, for the reasons given above.
      So instead of just expecting failure, let's also check for
      the magic word "ignoring" on stderr. That lets us know that
      we failed for the right reason.
      Signed-off-by: default avatarJeff King <peff@peff.net>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      f6adec4e
    • Jeff King's avatar
      submodule--helper: use "--" to signal end of clone options · 98afac7a
      Jeff King authored
      When we clone a submodule, we call "git clone $url $path".
      But there's nothing to say that those components can't begin
      with a dash themselves, confusing git-clone into thinking
      they're options. Let's pass "--" to make it clear what we
      expect.
      
      There's no test here, because it's actually quite hard to
      make these names work, even with "git clone" parsing them
      correctly. And we're going to restrict these cases even
      further in future commits. So we'll leave off testing until
      then; this is just the minimal fix to prevent us from doing
      something stupid with a badly formed entry.
      Reported-by: Joern Schneeweisz's avatarjoernchen <joernchen@phenoelit.de>
      Signed-off-by: default avatarJeff King <peff@peff.net>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      98afac7a
  2. 22 May, 2018 16 commits
    • Junio C Hamano's avatar
      Git 2.15.2 · d33c8751
      Junio C Hamano authored
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      d33c8751
    • Junio C Hamano's avatar
      Sync with Git 2.14.4 · 9e0f06d5
      Junio C Hamano authored
      * maint-2.14:
        Git 2.14.4
        Git 2.13.7
        verify_path: disallow symlinks in .gitmodules
        update-index: stat updated files earlier
        verify_dotfile: mention case-insensitivity in comment
        verify_path: drop clever fallthrough
        skip_prefix: add case-insensitive variant
        is_{hfs,ntfs}_dotgitmodules: add tests
        is_ntfs_dotgit: match other .git files
        is_hfs_dotgit: match other .git files
        is_ntfs_dotgit: use a size_t for traversing string
        submodule-config: verify submodule names as paths
      9e0f06d5
    • Junio C Hamano's avatar
      Git 2.14.4 · 4dde7b87
      Junio C Hamano authored
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      4dde7b87
    • Junio C Hamano's avatar
      Sync with Git 2.13.7 · 7b01c71b
      Junio C Hamano authored
      * maint-2.13:
        Git 2.13.7
        verify_path: disallow symlinks in .gitmodules
        update-index: stat updated files earlier
        verify_dotfile: mention case-insensitivity in comment
        verify_path: drop clever fallthrough
        skip_prefix: add case-insensitive variant
        is_{hfs,ntfs}_dotgitmodules: add tests
        is_ntfs_dotgit: match other .git files
        is_hfs_dotgit: match other .git files
        is_ntfs_dotgit: use a size_t for traversing string
        submodule-config: verify submodule names as paths
      7b01c71b
    • Junio C Hamano's avatar
      Git 2.13.7 · 0114f713
      Junio C Hamano authored
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      0114f713
    • Junio C Hamano's avatar
      Merge branch 'jk/submodule-fix-loose' into maint-2.13 · 8528c31d
      Junio C Hamano authored
      * jk/submodule-fix-loose:
        verify_path: disallow symlinks in .gitmodules
        update-index: stat updated files earlier
        verify_dotfile: mention case-insensitivity in comment
        verify_path: drop clever fallthrough
        skip_prefix: add case-insensitive variant
        is_{hfs,ntfs}_dotgitmodules: add tests
        is_ntfs_dotgit: match other .git files
        is_hfs_dotgit: match other .git files
        is_ntfs_dotgit: use a size_t for traversing string
        submodule-config: verify submodule names as paths
      8528c31d
    • Jeff King's avatar
      verify_path: disallow symlinks in .gitmodules · 10ecfa76
      Jeff King authored
      There are a few reasons it's not a good idea to make
      .gitmodules a symlink, including:
      
        1. It won't be portable to systems without symlinks.
      
        2. It may behave inconsistently, since Git may look at
           this file in the index or a tree without bothering to
           resolve any symbolic links. We don't do this _yet_, but
           the config infrastructure is there and it's planned for
           the future.
      
      With some clever code, we could make (2) work. And some
      people may not care about (1) if they only work on one
      platform. But there are a few security reasons to simply
      disallow it:
      
        a. A symlinked .gitmodules file may circumvent any fsck
           checks of the content.
      
        b. Git may read and write from the on-disk file without
           sanity checking the symlink target. So for example, if
           you link ".gitmodules" to "../oops" and run "git
           submodule add", we'll write to the file "oops" outside
           the repository.
      
      Again, both of those are problems that _could_ be solved
      with sufficient code, but given the complications in (1) and
      (2), we're better off just outlawing it explicitly.
      
      Note the slightly tricky call to verify_path() in
      update-index's update_one(). There we may not have a mode if
      we're not updating from the filesystem (e.g., we might just
      be removing the file). Passing "0" as the mode there works
      fine; since it's not a symlink, we'll just skip the extra
      checks.
      Signed-off-by: default avatarJeff King <peff@peff.net>
      10ecfa76
    • Jeff King's avatar
      update-index: stat updated files earlier · eb12dd0c
      Jeff King authored
      In the update_one(), we check verify_path() on the proposed
      path before doing anything else. In preparation for having
      verify_path() look at the file mode, let's stat the file
      earlier, so we can check the mode accurately.
      
      This is made a bit trickier by the fact that this function
      only does an lstat in a few code paths (the ones that flow
      down through process_path()). So we can speculatively do the
      lstat() here and pass the results down, and just use a dummy
      mode for cases where we won't actually be updating the index
      from the filesystem.
      Signed-off-by: default avatarJeff King <peff@peff.net>
      eb12dd0c
    • Jeff King's avatar
      verify_dotfile: mention case-insensitivity in comment · 641084b6
      Jeff King authored
      We're more restrictive than we need to be in matching ".GIT"
      on case-sensitive filesystems; let's make a note that this
      is intentional.
      Signed-off-by: default avatarJeff King <peff@peff.net>
      641084b6
    • Jeff King's avatar
      verify_path: drop clever fallthrough · e19e5e66
      Jeff King authored
      We check ".git" and ".." in the same switch statement, and
      fall through the cases to share the end-of-component check.
      While this saves us a line or two, it makes modifying the
      function much harder. Let's just write it out.
      Signed-off-by: default avatarJeff King <peff@peff.net>
      e19e5e66
    • Jeff King's avatar
      skip_prefix: add case-insensitive variant · 41a80924
      Jeff King authored
      We have the convenient skip_prefix() helper, but if you want
      to do case-insensitive matching, you're stuck doing it by
      hand. We could add an extra parameter to the function to
      let callers ask for this, but the function is small and
      somewhat performance-critical. Let's just re-implement it
      for the case-insensitive version.
      Signed-off-by: default avatarJeff King <peff@peff.net>
      41a80924
    • Johannes Schindelin's avatar
      is_{hfs,ntfs}_dotgitmodules: add tests · dc2d9ba3
      Johannes Schindelin authored
      This tests primarily for NTFS issues, but also adds one example of an
      HFS+ issue.
      
      Thanks go to Congyi Wu for coming up with the list of examples where
      NTFS would possibly equate the filename with `.gitmodules`.
      Signed-off-by: Johannes Schindelin's avatarJohannes Schindelin <johannes.schindelin@gmx.de>
      Signed-off-by: default avatarJeff King <peff@peff.net>
      dc2d9ba3
    • Johannes Schindelin's avatar
      is_ntfs_dotgit: match other .git files · e7cb0b44
      Johannes Schindelin authored
      When we started to catch NTFS short names that clash with .git, we only
      looked for GIT~1. This is sufficient because we only ever clone into an
      empty directory, so .git is guaranteed to be the first subdirectory or
      file in that directory.
      
      However, even with a fresh clone, .gitmodules is *not* necessarily the
      first file to be written that would want the NTFS short name GITMOD~1: a
      malicious repository can add .gitmodul0000 and friends, which sorts
      before `.gitmodules` and is therefore checked out *first*. For that
      reason, we have to test not only for ~1 short names, but for others,
      too.
      
      It's hard to just adapt the existing checks in is_ntfs_dotgit(): since
      Windows 2000 (i.e., in all Windows versions still supported by Git),
      NTFS short names are only generated in the <prefix>~<number> form up to
      number 4. After that, a *different* prefix is used, calculated from the
      long file name using an undocumented, but stable algorithm.
      
      For example, the short name of .gitmodules would be GITMOD~1, but if it
      is taken, and all of ~2, ~3 and ~4 are taken, too, the short name
      GI7EBA~1 will be used. From there, collisions are handled by
      incrementing the number, shortening the prefix as needed (until ~9999999
      is reached, in which case NTFS will not allow the file to be created).
      
      We'd also want to handle .gitignore and .gitattributes, which suffer
      from a similar problem, using the fall-back short names GI250A~1 and
      GI7D29~1, respectively.
      
      To accommodate for that, we could reimplement the hashing algorithm, but
      it is just safer and simpler to provide the known prefixes. This
      algorithm has been reverse-engineered and described at
      https://usn.pw/blog/gen/2015/06/09/filenames/, which is defunct but
      still available via https://web.archive.org/.
      
      These can be recomputed by running the following Perl script:
      
      -- snip --
      use warnings;
      use strict;
      
      sub compute_short_name_hash ($) {
              my $checksum = 0;
              foreach (split('', $_[0])) {
                      $checksum = ($checksum * 0x25 + ord($_)) & 0xffff;
              }
      
              $checksum = ($checksum * 314159269) & 0xffffffff;
              $checksum = 1 + (~$checksum & 0x7fffffff) if ($checksum & 0x80000000);
              $checksum -= (($checksum * 1152921497) >> 60) * 1000000007;
      
              return scalar reverse sprintf("%x", $checksum & 0xffff);
      }
      
      print compute_short_name_hash($ARGV[0]);
      -- snap --
      
      E.g., running that with the argument ".gitignore" will
      result in "250a" (which then becomes "gi250a" in the code).
      Signed-off-by: Johannes Schindelin's avatarJohannes Schindelin <johannes.schindelin@gmx.de>
      Signed-off-by: default avatarJeff King <peff@peff.net>
      e7cb0b44
    • Jeff King's avatar
      is_hfs_dotgit: match other .git files · 0fc333ba
      Jeff King authored
      Both verify_path() and fsck match ".git", ".GIT", and other
      variants specific to HFS+. Let's allow matching other
      special files like ".gitmodules", which we'll later use to
      enforce extra restrictions via verify_path() and fsck.
      Signed-off-by: default avatarJeff King <peff@peff.net>
      0fc333ba
    • Jeff King's avatar
      is_ntfs_dotgit: use a size_t for traversing string · 11a9f4d8
      Jeff King authored
      We walk through the "name" string using an int, which can
      wrap to a negative value and cause us to read random memory
      before our array (e.g., by creating a tree with a name >2GB,
      since "int" is still 32 bits even on most 64-bit platforms).
      Worse, this is easy to trigger during the fsck_tree() check,
      which is supposed to be protecting us from malicious
      garbage.
      
      Note one bit of trickiness in the existing code: we
      sometimes assign -1 to "len" at the end of the loop, and
      then rely on the "len++" in the for-loop's increment to take
      it back to 0. This is still legal with a size_t, since
      assigning -1 will turn into SIZE_MAX, which then wraps
      around to 0 on increment.
      Signed-off-by: default avatarJeff King <peff@peff.net>
      11a9f4d8
    • 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
  3. 06 Dec, 2017 13 commits
  4. 28 Nov, 2017 2 commits
  5. 27 Nov, 2017 3 commits