fix(repo): clamp WithMaxObjectSize to MaxInt; return refs/replace errors (GO-011, GO-013)
WithMaxObjectSize is capped at math.MaxInt so every int(size) conversion is safe on 32-bit platforms, and grow no longer overflows computing its spare capacity. checkLayout returns errors other than ENOENT/ENOTDIR when reading refs/replace instead of swallowing them.
- commit
- 3c103d35ead62945b121a968267cb43c28922aea
- author
- brice · · 2026-10-10 22:15 UTC
- parent
- 6881eb3
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +89 -14
M inflate.go +8 -1
| @@ -8,6 +8,7 @@ import ( | |||
| 8 | 8 | "compress/zlib" | |
| 9 | 9 | "errors" | |
| 10 | 10 | "io" | |
| 11 | + | "math" | |
| 11 | 12 | "sync" | |
| 12 | 13 | ) | |
| 13 | 14 | ||
| @@ -37,7 +38,13 @@ func grow(b []byte, n int) []byte { | |||
| 37 | 38 | if cap(b) >= n { | |
| 38 | 39 | return b[:n] | |
| 39 | 40 | } | |
| 40 | - | return make([]byte, n, n+n/4) | |
| 41 | + | // Spare room for the next, slightly bigger object; n+n/4 may overflow | |
| 42 | + | // an int on 32-bit systems, where the size is capped at MaxInt. | |
| 43 | + | c := n | |
| 44 | + | if n <= math.MaxInt-n/4 { | |
| 45 | + | c = n + n/4 | |
| 46 | + | } | |
| 47 | + | return make([]byte, n, c) | |
| 41 | 48 | } | |
| 42 | 49 | ||
| 43 | 50 | // memInflater inflates a zlib stream held in memory. | |
| @@ -8,6 +8,7 @@ import ( | |||||
| 8 | "compress/zlib" | 8 | "compress/zlib" | ||
| 9 | "errors" | 9 | "errors" | ||
| 10 | "io" | 10 | "io" | ||
| 11 | + | "math" | |||
| 11 | "sync" | 12 | "sync" | ||
| 12 | ) | 13 | ) | ||
| 13 | 14 | ||||
| @@ -37,7 +38,13 @@ func grow(b []byte, n int) []byte { | |||||
| 37 | if cap(b) >= n { | 38 | if cap(b) >= n { | ||
| 38 | return b[:n] | 39 | return b[:n] | ||
| 39 | } | 40 | } | ||
| 40 | - | return make([]byte, n, n+n/4) | 41 | + | // Spare room for the next, slightly bigger object; n+n/4 may overflow |
| 42 | + | // an int on 32-bit systems, where the size is capped at MaxInt. | |||
| 43 | + | c := n | |||
| 44 | + | if n <= math.MaxInt-n/4 { | |||
| 45 | + | c = n + n/4 | |||
| 46 | + | } | |||
| 47 | + | return make([]byte, n, c) | |||
| 41 | } | 48 | } | ||
| 42 | 49 | ||||
| 43 | // memInflater inflates a zlib stream held in memory. | 50 | // memInflater inflates a zlib stream held in memory. | ||
M repo.go +25 -13
| @@ -4,11 +4,14 @@ package git | |||
| 4 | 4 | ||
| 5 | 5 | import ( | |
| 6 | 6 | "errors" | |
| 7 | + | "io" | |
| 7 | 8 | "io/fs" | |
| 9 | + | "math" | |
| 8 | 10 | "os" | |
| 9 | 11 | "path/filepath" | |
| 10 | 12 | "sync" | |
| 11 | 13 | "sync/atomic" | |
| 14 | + | "syscall" | |
| 12 | 15 | ) | |
| 13 | 16 | ||
| 14 | 17 | // Default bounds. See WithMaxObjectSize and WithDeltaCacheSize. | |
| @@ -31,11 +34,12 @@ type Option func(*options) | |||
| 31 | 34 | // WithMaxObjectSize bounds the size of an object that is held in memory: | |
| 32 | 35 | // commits, trees, tags and blobs that are stored as deltas. Bigger ones fail | |
| 33 | 36 | // with ErrTooLarge. Undeltified blobs stream and are not bounded. The default | |
| 34 | - | // is 128 MiB. | |
| 37 | + | // is 128 MiB. The bound is capped at the largest int of the platform | |
| 38 | + | // (2 GiB - 1 on 32-bit systems), since such an object must fit in a slice. | |
| 35 | 39 | func WithMaxObjectSize(n int64) Option { | |
| 36 | 40 | return func(o *options) { | |
| 37 | 41 | if n > 0 { | |
| 38 | - | o.maxObject = n | |
| 42 | + | o.maxObject = min(n, math.MaxInt) | |
| 39 | 43 | } | |
| 40 | 44 | } | |
| 41 | 45 | } | |
| @@ -176,6 +180,12 @@ func isGitDir(dir string) bool { | |||
| 176 | 180 | return true | |
| 177 | 181 | } | |
| 178 | 182 | ||
| 183 | + | // isNotExist reports whether err means that a path, or a directory on the way | |
| 184 | + | // to it, is absent. | |
| 185 | + | func isNotExist(err error) bool { | |
| 186 | + | return errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) | |
| 187 | + | } | |
| 188 | + | ||
| 179 | 189 | func exists(path string) bool { | |
| 180 | 190 | _, err := os.Lstat(path) | |
| 181 | 191 | return err == nil | |
| @@ -195,17 +205,19 @@ func checkLayout(dir string) error { | |||
| 195 | 205 | } | |
| 196 | 206 | } | |
| 197 | 207 | f, err := os.Open(filepath.Join(dir, "refs", "replace")) | |
| 198 | - | if err == nil { | |
| 199 | - | names, rerr := f.Readdirnames(1) | |
| 200 | - | _ = f.Close() | |
| 201 | - | if rerr == nil && len(names) > 0 { | |
| 202 | - | return unsupported("replace refs (refs/replace)") | |
| 203 | - | } | |
| 204 | - | } else if !errors.Is(err, fs.ErrNotExist) && !errors.Is(err, fs.ErrInvalid) { | |
| 205 | - | var pe *fs.PathError | |
| 206 | - | if !errors.As(err, &pe) { | |
| 207 | - | return err | |
| 208 | + | if err != nil { | |
| 209 | + | if isNotExist(err) { | |
| 210 | + | return nil | |
| 208 | 211 | } | |
| 212 | + | return err | |
| 209 | 213 | } | |
| 210 | - | return nil | |
| 214 | + | names, rerr := f.Readdirnames(1) | |
| 215 | + | _ = f.Close() | |
| 216 | + | switch { | |
| 217 | + | case rerr == nil && len(names) > 0: | |
| 218 | + | return unsupported("replace refs (refs/replace)") | |
| 219 | + | case rerr == nil, errors.Is(rerr, io.EOF), isNotExist(rerr): | |
| 220 | + | return nil | |
| 221 | + | } | |
| 222 | + | return rerr | |
| 211 | 223 | } | |
| @@ -4,11 +4,14 @@ package git | |||||
| 4 | 4 | ||||
| 5 | import ( | 5 | import ( | ||
| 6 | "errors" | 6 | "errors" | ||
| 7 | + | "io" | |||
| 7 | "io/fs" | 8 | "io/fs" | ||
| 9 | + | "math" | |||
| 8 | "os" | 10 | "os" | ||
| 9 | "path/filepath" | 11 | "path/filepath" | ||
| 10 | "sync" | 12 | "sync" | ||
| 11 | "sync/atomic" | 13 | "sync/atomic" | ||
| 14 | + | "syscall" | |||
| 12 | ) | 15 | ) | ||
| 13 | 16 | ||||
| 14 | // Default bounds. See WithMaxObjectSize and WithDeltaCacheSize. | 17 | // Default bounds. See WithMaxObjectSize and WithDeltaCacheSize. | ||
| @@ -31,11 +34,12 @@ type Option func(*options) | |||||
| 31 | // WithMaxObjectSize bounds the size of an object that is held in memory: | 34 | // WithMaxObjectSize bounds the size of an object that is held in memory: | ||
| 32 | // commits, trees, tags and blobs that are stored as deltas. Bigger ones fail | 35 | // commits, trees, tags and blobs that are stored as deltas. Bigger ones fail | ||
| 33 | // with ErrTooLarge. Undeltified blobs stream and are not bounded. The default | 36 | // with ErrTooLarge. Undeltified blobs stream and are not bounded. The default | ||
| 34 | - | // is 128 MiB. | 37 | + | // is 128 MiB. The bound is capped at the largest int of the platform |
| 38 | + | // (2 GiB - 1 on 32-bit systems), since such an object must fit in a slice. | |||
| 35 | func WithMaxObjectSize(n int64) Option { | 39 | func WithMaxObjectSize(n int64) Option { | ||
| 36 | return func(o *options) { | 40 | return func(o *options) { | ||
| 37 | if n > 0 { | 41 | if n > 0 { | ||
| 38 | - | o.maxObject = n | 42 | + | o.maxObject = min(n, math.MaxInt) |
| 39 | } | 43 | } | ||
| 40 | } | 44 | } | ||
| 41 | } | 45 | } | ||
| @@ -176,6 +180,12 @@ func isGitDir(dir string) bool { | |||||
| 176 | return true | 180 | return true | ||
| 177 | } | 181 | } | ||
| 178 | 182 | ||||
| 183 | + | // isNotExist reports whether err means that a path, or a directory on the way | |||
| 184 | + | // to it, is absent. | |||
| 185 | + | func isNotExist(err error) bool { | |||
| 186 | + | return errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) | |||
| 187 | + | } | |||
| 188 | + | ||||
| 179 | func exists(path string) bool { | 189 | func exists(path string) bool { | ||
| 180 | _, err := os.Lstat(path) | 190 | _, err := os.Lstat(path) | ||
| 181 | return err == nil | 191 | return err == nil | ||
| @@ -195,17 +205,19 @@ func checkLayout(dir string) error { | |||||
| 195 | } | 205 | } | ||
| 196 | } | 206 | } | ||
| 197 | f, err := os.Open(filepath.Join(dir, "refs", "replace")) | 207 | f, err := os.Open(filepath.Join(dir, "refs", "replace")) | ||
| 198 | - | if err == nil { | 208 | + | if err != nil { |
| 199 | - | names, rerr := f.Readdirnames(1) | 209 | + | if isNotExist(err) { |
| 200 | - | _ = f.Close() | 210 | + | return nil |
| 201 | - | if rerr == nil && len(names) > 0 { | |||
| 202 | - | return unsupported("replace refs (refs/replace)") | |||
| 203 | - | } | |||
| 204 | - | } else if !errors.Is(err, fs.ErrNotExist) && !errors.Is(err, fs.ErrInvalid) { | |||
| 205 | - | var pe *fs.PathError | |||
| 206 | - | if !errors.As(err, &pe) { | |||
| 207 | - | return err | |||
| 208 | } | 211 | } | ||
| 212 | + | return err | |||
| 209 | } | 213 | } | ||
| 210 | - | return nil | 214 | + | names, rerr := f.Readdirnames(1) |
| 215 | + | _ = f.Close() | |||
| 216 | + | switch { | |||
| 217 | + | case rerr == nil && len(names) > 0: | |||
| 218 | + | return unsupported("replace refs (refs/replace)") | |||
| 219 | + | case rerr == nil, errors.Is(rerr, io.EOF), isNotExist(rerr): | |||
| 220 | + | return nil | |||
| 221 | + | } | |||
| 222 | + | return rerr | |||
| 211 | } | 223 | } | ||
A repo_fix_test.go +56 -0
| @@ -0,0 +1,56 @@ | |||
| 1 | + | // SPDX-License-Identifier: MIT | |
| 2 | + | ||
| 3 | + | package git | |
| 4 | + | ||
| 5 | + | import ( | |
| 6 | + | "errors" | |
| 7 | + | "math" | |
| 8 | + | "os" | |
| 9 | + | "testing" | |
| 10 | + | ) | |
| 11 | + | ||
| 12 | + | func TestWithMaxObjectSizeClamped(t *testing.T) { | |
| 13 | + | var o options | |
| 14 | + | WithMaxObjectSize(math.MaxInt64)(&o) | |
| 15 | + | if o.maxObject != math.MaxInt { | |
| 16 | + | t.Errorf("maxObject = %d, want %d", o.maxObject, math.MaxInt) | |
| 17 | + | } | |
| 18 | + | WithMaxObjectSize(1 << 20)(&o) | |
| 19 | + | if o.maxObject != 1<<20 { | |
| 20 | + | t.Errorf("maxObject = %d", o.maxObject) | |
| 21 | + | } | |
| 22 | + | WithMaxObjectSize(-5)(&o) | |
| 23 | + | if o.maxObject != 1<<20 { | |
| 24 | + | t.Errorf("a negative bound changed it to %d", o.maxObject) | |
| 25 | + | } | |
| 26 | + | } | |
| 27 | + | ||
| 28 | + | func TestReplaceDirErrorsAreReturned(t *testing.T) { | |
| 29 | + | if os.Geteuid() == 0 { | |
| 30 | + | t.Skip("root reads any directory") | |
| 31 | + | } | |
| 32 | + | f := newFx(t) | |
| 33 | + | f.write("a", []byte("a"), 0o644) | |
| 34 | + | f.commit("one") | |
| 35 | + | dir := f.dir + "/refs/replace" | |
| 36 | + | if err := os.MkdirAll(dir, 0o755); err != nil { | |
| 37 | + | t.Fatal(err) | |
| 38 | + | } | |
| 39 | + | if err := os.Chmod(dir, 0); err != nil { | |
| 40 | + | t.Fatal(err) | |
| 41 | + | } | |
| 42 | + | t.Cleanup(func() { _ = os.Chmod(dir, 0o755) }) | |
| 43 | + | if _, err := Open(f.work); !errors.Is(err, os.ErrPermission) { | |
| 44 | + | t.Fatalf("Open with an unreadable refs/replace = %v, want a permission error", err) | |
| 45 | + | } | |
| 46 | + | // Absent, or not a directory: nothing to refuse. | |
| 47 | + | _ = os.Chmod(dir, 0o755) | |
| 48 | + | _ = os.Remove(dir) | |
| 49 | + | if err := checkLayout(f.dir); err != nil { | |
| 50 | + | t.Errorf("no refs/replace: %v", err) | |
| 51 | + | } | |
| 52 | + | writeFile(t, dir, "a file") | |
| 53 | + | if err := checkLayout(f.dir); err != nil { | |
| 54 | + | t.Errorf("refs/replace is a file: %v", err) | |
| 55 | + | } | |
| 56 | + | } | |
| @@ -0,0 +1,56 @@ | |||||
| 1 | + | // SPDX-License-Identifier: MIT | |||
| 2 | + | ||||
| 3 | + | package git | |||
| 4 | + | ||||
| 5 | + | import ( | |||
| 6 | + | "errors" | |||
| 7 | + | "math" | |||
| 8 | + | "os" | |||
| 9 | + | "testing" | |||
| 10 | + | ) | |||
| 11 | + | ||||
| 12 | + | func TestWithMaxObjectSizeClamped(t *testing.T) { | |||
| 13 | + | var o options | |||
| 14 | + | WithMaxObjectSize(math.MaxInt64)(&o) | |||
| 15 | + | if o.maxObject != math.MaxInt { | |||
| 16 | + | t.Errorf("maxObject = %d, want %d", o.maxObject, math.MaxInt) | |||
| 17 | + | } | |||
| 18 | + | WithMaxObjectSize(1 << 20)(&o) | |||
| 19 | + | if o.maxObject != 1<<20 { | |||
| 20 | + | t.Errorf("maxObject = %d", o.maxObject) | |||
| 21 | + | } | |||
| 22 | + | WithMaxObjectSize(-5)(&o) | |||
| 23 | + | if o.maxObject != 1<<20 { | |||
| 24 | + | t.Errorf("a negative bound changed it to %d", o.maxObject) | |||
| 25 | + | } | |||
| 26 | + | } | |||
| 27 | + | ||||
| 28 | + | func TestReplaceDirErrorsAreReturned(t *testing.T) { | |||
| 29 | + | if os.Geteuid() == 0 { | |||
| 30 | + | t.Skip("root reads any directory") | |||
| 31 | + | } | |||
| 32 | + | f := newFx(t) | |||
| 33 | + | f.write("a", []byte("a"), 0o644) | |||
| 34 | + | f.commit("one") | |||
| 35 | + | dir := f.dir + "/refs/replace" | |||
| 36 | + | if err := os.MkdirAll(dir, 0o755); err != nil { | |||
| 37 | + | t.Fatal(err) | |||
| 38 | + | } | |||
| 39 | + | if err := os.Chmod(dir, 0); err != nil { | |||
| 40 | + | t.Fatal(err) | |||
| 41 | + | } | |||
| 42 | + | t.Cleanup(func() { _ = os.Chmod(dir, 0o755) }) | |||
| 43 | + | if _, err := Open(f.work); !errors.Is(err, os.ErrPermission) { | |||
| 44 | + | t.Fatalf("Open with an unreadable refs/replace = %v, want a permission error", err) | |||
| 45 | + | } | |||
| 46 | + | // Absent, or not a directory: nothing to refuse. | |||
| 47 | + | _ = os.Chmod(dir, 0o755) | |||
| 48 | + | _ = os.Remove(dir) | |||
| 49 | + | if err := checkLayout(f.dir); err != nil { | |||
| 50 | + | t.Errorf("no refs/replace: %v", err) | |||
| 51 | + | } | |||
| 52 | + | writeFile(t, dir, "a file") | |||
| 53 | + | if err := checkLayout(f.dir); err != nil { | |||
| 54 | + | t.Errorf("refs/replace is a file: %v", err) | |||
| 55 | + | } | |||
| 56 | + | } | |||