fix: validate object fields before use

blob.go

@@ -2,6 +2,7 @@ package ipldgit
 
 import (
 	"bufio"
+	"bytes"
 	"fmt"
 	"io"
 
@@ -14,21 +15,32 @@ func DecodeBlob(na ipld.NodeAssembler, rd *bufio.Reader) error {
 	if err != nil {
 		return err
 	}
+	if sizen < 0 {
+		return fmt.Errorf("invalid blob size: %d", sizen)
+	}
 
 	prefix := fmt.Sprintf("blob %d\x00", sizen)
-	buf := make([]byte, len(prefix)+sizen)
-	copy(buf, prefix)
 
-	n, err := io.ReadFull(rd, buf[len(prefix):])
+	// The header's size is unverified until the body arrives, so grow the
+	// buffer as it is read rather than reserving the declared size up front.
+	var buf bytes.Buffer
+	buf.WriteString(prefix)
+
+	n, err := io.Copy(&buf, io.LimitReader(rd, int64(sizen)))
 	if err != nil {
 		return err
 	}
 
-	if n != sizen {
-		return fmt.Errorf("blob size was not accurate")
+	// Match io.ReadFull: EOF if the body was entirely absent, ErrUnexpectedEOF
+	// if it was short.
+	if n != int64(sizen) {
+		if n == 0 {
+			return io.EOF
+		}
+		return io.ErrUnexpectedEOF
 	}
 
-	return na.AssignBytes(buf)
+	return na.AssignBytes(buf.Bytes())
 }
 
 func encodeBlob(n ipld.Node, w io.Writer) error {

commit.go

@@ -50,14 +50,24 @@ func decodeCommitLine(c Commit, line []byte, rd *bufio.Reader) error {
 			return err
 		}
 
-		c.tree = _Tree_Link{cidlink.Link{Cid: shaToCid(sha)}}
+		treeCid, err := shaToCid(sha)
+		if err != nil {
+			return err
+		}
+
+		c.tree = _Tree_Link{cidlink.Link{Cid: treeCid}}
 	case bytes.HasPrefix(line, []byte("parent ")):
 		psha, err := hex.DecodeString(string(line[7:]))
 		if err != nil {
 			return err
 		}
 
-		c.parents.x = append(c.parents.x, _Commit_Link{cidlink.Link{Cid: shaToCid(psha)}})
+		parentCid, err := shaToCid(psha)
+		if err != nil {
+			return err
+		}
+
+		c.parents.x = append(c.parents.x, _Commit_Link{cidlink.Link{Cid: parentCid}})
 	case bytes.HasPrefix(line, []byte("author ")):
 		a, err := parsePersonInfo(line)
 		if err != nil {
@@ -120,29 +130,35 @@ func decodeGpgSig(rd *bufio.Reader) (_GpgSig, error) {
 		return out, err
 	}
 
+	// Accumulate in a builder: the signature has no fixed size, and repeated
+	// string concatenation would copy the whole thing on every line.
+	var sig strings.Builder
+
 	if string(line) != " " {
 		if strings.HasPrefix(string(line), " Version: ") || strings.HasPrefix(string(line), " Comment: ") {
-			out.x += string(line) + "\n"
+			sig.WriteString(string(line) + "\n")
 		} else {
 			return out, fmt.Errorf("expected first line of sig to be a single space or version")
 		}
 	} else {
-		out.x += " \n"
+		sig.WriteString(" \n")
 	}
 
 	for {
 		line, _, err := rd.ReadLine()
 		if err != nil {
+			out.x = sig.String()
 			return out, err
 		}
 
 		if bytes.Equal(line, []byte(" -----END PGP SIGNATURE-----")) {
 			break
 		}
 
-		out.x += string(line) + "\n"
+		sig.WriteString(string(line) + "\n")
 	}
 
+	out.x = sig.String()
 	return out, nil
 }
 

git_test.go

@@ -6,6 +6,7 @@ import (
 	"compress/gzip"
 	"fmt"
 	"io"
+	"io/fs"
 	"os"
 	"path/filepath"
 	"strings"
@@ -28,21 +29,23 @@ func TestObjectParse(t *testing.T) {
 	ls := cidlink.DefaultLinkSystem()
 
 	i := 0
-	err := filepath.Walk(".git/objects", func(path string, info os.FileInfo, err error) error {
+	err := filepath.WalkDir(".git/objects", func(path string, d fs.DirEntry, err error) error {
 		if err != nil {
 			return err
 		}
-		if info.IsDir() {
+		if d.IsDir() {
+			// Only loose object files should reach the code below. Skip the
+			// pack and info directories and everything inside them, because
+			// git keeps other kinds of files there, such as a commit-graph
+			// tucked in a folder under info.
+			if name := d.Name(); name == "info" || name == "pack" {
+				return filepath.SkipDir
+			}
 			return nil
 		}
 
 		parts := strings.Split(path, string(filepath.Separator))
 
-		dir := parts[len(parts)-2]
-		if dir == "info" || dir == "pack" {
-			return nil
-		}
-
 		fi, err := os.Open(path)
 		if err != nil {
 			return err
@@ -407,16 +410,16 @@ func assert(t *testing.T, ok bool) {
 
 func BenchmarkRawData(b *testing.B) {
 	for i := 0; i < b.N; i++ {
-		err := filepath.Walk(".git/objects", func(path string, info os.FileInfo, err error) error {
+		err := filepath.WalkDir(".git/objects", func(path string, d fs.DirEntry, err error) error {
 			if err != nil {
 				return nil
 			}
-			if info.IsDir() {
-				return nil
-			}
-
-			parts := strings.Split(path, string(filepath.Separator))
-			if dir := parts[len(parts)-2]; dir == "info" || dir == "pack" {
+			if d.IsDir() {
+				// See TestObjectParse: git keeps files that are not loose
+				// objects under pack and info.
+				if name := d.Name(); name == "info" || name == "pack" {
+					return filepath.SkipDir
+				}
 				return nil
 			}
 
@@ -448,16 +451,16 @@ func BenchmarkCid(b *testing.B) {
 	ls := cidlink.DefaultLinkSystem()
 
 	for i := 0; i < b.N; i++ {
-		err := filepath.Walk(".git/objects", func(path string, info os.FileInfo, err error) error {
+		err := filepath.WalkDir(".git/objects", func(path string, d fs.DirEntry, err error) error {
 			if err != nil {
 				return nil
 			}
-			if info.IsDir() {
-				return nil
-			}
-
-			parts := strings.Split(path, string(filepath.Separator))
-			if dir := parts[len(parts)-2]; dir == "info" || dir == "pack" {
+			if d.IsDir() {
+				// See TestObjectParse: git keeps files that are not loose
+				// objects under pack and info.
+				if name := d.Name(); name == "info" || name == "pack" {
+					return filepath.SkipDir
+				}
 				return nil
 			}
 

personinfo.go

@@ -44,12 +44,15 @@ func parsePersonInfo(line []byte) (PersonInfo, error) {
 			return nil, fmt.Errorf("invalid personInfo: %q", line)
 		}
 		part := parts[at]
-		if part[0] == '<' {
+		// A part can be empty when the email contains repeated spaces, which
+		// git itself produces, so skip rather than index into it. The name loop
+		// above already treats that case the same way.
+		if len(part) > 0 && part[0] == '<' {
 			part = part[1:]
 		}
 
 		at++
-		if part[len(part)-1] == '>' {
+		if len(part) > 0 && part[len(part)-1] == '>' {
 			email.WriteString(string(part[:len(part)-1]))
 			break
 		}

tag.go

@@ -6,6 +6,7 @@ import (
 	"encoding/hex"
 	"fmt"
 	"io"
+	"strings"
 
 	"github.com/ipld/go-ipld-prime"
 	cidlink "github.com/ipld/go-ipld-prime/linking/cid"
@@ -40,7 +41,11 @@ func DecodeTag(na ipld.NodeAssembler, rd *bufio.Reader) error {
 				return err
 			}
 
-			out.object = _Link{cidlink.Link{Cid: shaToCid(sha)}}
+			c, err := shaToCid(sha)
+			if err != nil {
+				return err
+			}
+			out.object = _Link{cidlink.Link{Cid: c}}
 		case bytes.HasPrefix(line, []byte("tag ")):
 			out.tag = _String{string(line[tagTagPrefixLen:])}
 		case bytes.HasPrefix(line, []byte("tagger ")):
@@ -71,7 +76,11 @@ func DecodeTag(na ipld.NodeAssembler, rd *bufio.Reader) error {
 func readMergeTag(hash []byte, rd *bufio.Reader) (Tag, []byte, error) {
 	out := _Tag{}
 
-	out.object = _Link{cidlink.Link{Cid: shaToCid(hash)}}
+	objCid, err := shaToCid(hash)
+	if err != nil {
+		return nil, nil, err
+	}
+	out.object = _Link{cidlink.Link{Cid: objCid}}
 	for {
 		line, _, err := rd.ReadLine()
 		if err != nil {
@@ -93,17 +102,22 @@ func readMergeTag(hash []byte, rd *bufio.Reader) (Tag, []byte, error) {
 			}
 			out.tagger = *tagger
 		case string(line) == " ":
+			// Accumulate in a builder: the message has no fixed size, and
+			// repeated string concatenation would copy it on every line.
+			var msg strings.Builder
 			for {
 				line, _, err := rd.ReadLine()
 				if err != nil {
+					out.message.x = msg.String()
 					return nil, nil, err
 				}
 
 				if !bytes.HasPrefix(line, []byte(" ")) {
+					out.message.x = msg.String()
 					return &out, line, nil
 				}
 
-				out.message.x += string(line) + "\n"
+				msg.WriteString(string(line) + "\n")
 			}
 		}
 	}

tree.go

@@ -63,9 +63,14 @@ func DecodeTreeEntry(rd *bufio.Reader) (string, ipld.Node, error) {
 		return "", nil, err
 	}
 
+	c, err := shaToCid(sha)
+	if err != nil {
+		return "", nil, err
+	}
+
 	te := _TreeEntry{
 		mode: _String{data},
-		hash: _Link{cidlink.Link{Cid: shaToCid(sha)}},
+		hash: _Link{cidlink.Link{Cid: c}},
 	}
 	return name, &te, nil
 }

util.go

@@ -1,15 +1,28 @@
 package ipldgit
 
 import (
+	"fmt"
+
 	"github.com/ipfs/go-cid"
 	"github.com/ipld/go-ipld-prime"
 	cidlink "github.com/ipld/go-ipld-prime/linking/cid"
 	mh "github.com/multiformats/go-multihash"
 )
 
-func shaToCid(sha []byte) cid.Cid {
-	h, _ := mh.Encode(sha, mh.SHA1)
-	return cid.NewCidV1(cid.GitRaw, h)
+// gitSHALen is the length of a git object hash. A reference of any other length
+// cannot name a git object, and cidToSha assumes this width when it turns a CID
+// back into one.
+const gitSHALen = 20
+
+func shaToCid(sha []byte) (cid.Cid, error) {
+	if len(sha) != gitSHALen {
+		return cid.Undef, fmt.Errorf("invalid git sha of %d bytes, expected %d", len(sha), gitSHALen)
+	}
+	h, err := mh.Encode(sha, mh.SHA1)
+	if err != nil {
+		return cid.Undef, err
+	}
+	return cid.NewCidV1(cid.GitRaw, h), nil
 }
 
 func cidToSha(c cid.Cid) []byte {

version.json

@@ -1,3 +1,3 @@
 {
-  "version": "v0.1.2"
+  "version": "v0.1.3"
 }