1. 05 Jul, 2018 1 commit
    • delta: fix sign-extension of big left-shift · 3f461902
      Our delta code was originally adapted from JGit, which itself adapted it
      from git itself. Due to this heritage, we inherited a bug from git.git
      in how we compute the delta offset, which was fixed upstream in
      48fb7deb5 (Fix big left-shifts of unsigned char, 2009-06-17). As
      explained by Linus:
      
          Shifting 'unsigned char' or 'unsigned short' left can result in sign
          extension errors, since the C integer promotion rules means that the
          unsigned char/short will get implicitly promoted to a signed 'int' due to
          the shift (or due to other operations).
      
          This normally doesn't matter, but if you shift things up sufficiently, it
          will now set the sign bit in 'int', and a subsequent cast to a bigger type
          (eg 'long' or 'unsigned long') will now sign-extend the value despite the
          original expression being unsigned.
      
          One example of this would be something like
      
                  unsigned long size;
                  unsigned char c;
      
                  size += c << 24;
      
          where despite all the variables being unsigned, 'c << 24' ends up being a
          signed entity, and will get sign-extended when then doing the addition in
          an 'unsigned long' type.
      
          Since git uses 'unsigned char' pointers extensively, we actually have this
          bug in a couple of places.
      
      In our delta code, we inherited such a bogus shift when computing the
      offset at which the delta base is to be found. Due to the sign extension
      we can end up with an offset where all the bits are set. This can allow
      an arbitrary memory read, as the addition in `base_len < off + len` can
      now overflow if `off` has all its bits set.
      
      Fix the issue by casting the result of `*delta++ << 24UL` to an unsigned
      integer again. Add a test with a crafted delta that would actually
      succeed with an out-of-bounds read in case where the cast wouldn't
      exist.
      
      Reported-by: Riccardo Schirone <rschiron@redhat.com>
      Test-provided-by: Riccardo Schirone <rschiron@redhat.com>
      Patrick Steinhardt committed
  2. 10 Jun, 2018 1 commit
  3. 06 Jun, 2018 4 commits
    • tests: submodule: do not rely on config iteration order · 35865117
      The test submodule::lookup::duplicated_path, which tries to verify that
      we detect submodules with duplicated paths, currently relies on the
      gitmodules file of "submod2_target". While this file has two gitmodules
      with the same path, one of these gitmodules has an empty name and thus
      does not pass `git_submodule_name_is_valid`. Because of this, the test
      is in fact dependent on the iteration order in which we process the
      submodules. In fact the "valid" submodule comes first, the "invalid"
      submodule will cause the desired error. In fact the "invalid" submodule
      comes first, it will be skipped due to its name being invalid, and we
      will not see the desired error. While this works on the master branch
      just right due to the refactoring of our config code, where iteration
      order is now deterministic, this breaks on all older maintenance
      branches.
      
      Fix the issue by simply using `cl_git_rewritefile` to rewrite the
      gitmodules file. This greatly simplifies the test and also makes the
      intentions of it much clearer.
      Patrick Steinhardt committed
    • submodule: detect duplicated submodule paths · 7392799d
      When loading submodule names, we build a map of submodule paths and
      their respective names. While looping over the configuration keys,
      we do not check though whether a submodule path was seen already. This
      leads to a memory leak in case we have multiple submodules with the same
      path, as we just overwrite the old value in the map in that case.
      
      Fix the error by verifying that the path to be added is not yet part of
      the string map. Git does not allow to have multiple submodules for a
      path anyway, so we now do the same and detect this duplication,
      reporting it to the user.
      Patrick Steinhardt committed
  4. 30 May, 2018 24 commits
  5. 29 May, 2018 10 commits