diff --git a/cmd/seccat/util.go b/cmd/seccat/util.go index c458f023114..d2188353537 100644 --- a/cmd/seccat/util.go +++ b/cmd/seccat/util.go @@ -31,7 +31,7 @@ type logRW struct { func (r *logRW) Read(buf []byte) (int, error) { n, err := r.rw.Read(buf) - if err == nil { + if n > 0 { log.Debugf("%s read: %v", r.n, buf) } return n, err diff --git a/repo/fsrepo/fsrepo.go b/repo/fsrepo/fsrepo.go index 181f59c19b2..f4d1e37c406 100644 --- a/repo/fsrepo/fsrepo.go +++ b/repo/fsrepo/fsrepo.go @@ -324,13 +324,21 @@ func APIAddr(repoPath string) (ma.Multiaddr, error) { // read up to 2048 bytes. io.ReadAll is a vulnerability, as // someone could hose the process by putting a massive file there. - buf := make([]byte, 2048) - n, err := f.Read(buf) - if err != nil && err != io.EOF { + // + // NOTE(@stebalien): @jbenet probably wasn't thinking straight when he + // wrote that comment but I'm leaving the limit here in case there was + // some hidden wisdom. However, I'm fixing it such that: + // 1. We don't read too little. + // 2. We don't truncate and succeed. + buf, err := ioutil.ReadAll(io.LimitReader(f, 2048)) + if err != nil { return nil, err } + if len(buf) == 2048 { + return nil, fmt.Errorf("API file too large, must be <2048 bytes long: %s", apiFilePath) + } - s := string(buf[:n]) + s := string(buf) s = strings.TrimSpace(s) return ma.NewMultiaddr(s) } diff --git a/unixfs/io/pbdagreader.go b/unixfs/io/pbdagreader.go index ce3abf439ff..5be60bd8e14 100644 --- a/unixfs/io/pbdagreader.go +++ b/unixfs/io/pbdagreader.go @@ -166,22 +166,20 @@ func (dr *PBDagReader) CtxReadFull(ctx context.Context, b []byte) (int, error) { total := 0 for { // Attempt to fill bytes from cached buffer - n, err := dr.buf.Read(b[total:]) + n, err := io.ReadFull(dr.buf, b[total:]) total += n dr.offset += int64(n) - if err != nil { - // EOF is expected - if err != io.EOF { - return total, err - } - } - - // If weve read enough bytes, return - if total == len(b) { + switch err { + // io.EOF will happen is dr.buf had noting more to read (n == 0) + case io.EOF, io.ErrUnexpectedEOF: + // do nothing + case nil: return total, nil + default: + return total, err } - // Otherwise, load up the next block + // if we are not done with the output buffer load next block err = dr.precalcNextBuf(ctx) if err != nil { return total, err diff --git a/unixfs/mod/dagmodifier.go b/unixfs/mod/dagmodifier.go index 8f2766aee05..0fe9df1e4e1 100644 --- a/unixfs/mod/dagmodifier.go +++ b/unixfs/mod/dagmodifier.go @@ -202,7 +202,7 @@ func (dm *DagModifier) Sync() error { buflen := dm.wrBuf.Len() // overwrite existing dag nodes - thisc, done, err := dm.modifyDag(dm.curNode, dm.writeStart, dm.wrBuf) + thisc, err := dm.modifyDag(dm.curNode, dm.writeStart) if err != nil { return err } @@ -213,7 +213,7 @@ func (dm *DagModifier) Sync() error { } // need to write past end of current dag - if !done { + if dm.wrBuf.Len() > 0 { dm.curNode, err = dm.appendData(dm.curNode, dm.splitter(dm.wrBuf)) if err != nil { return err @@ -231,28 +231,27 @@ func (dm *DagModifier) Sync() error { return nil } -// modifyDag writes the data in 'data' over the data in 'node' starting at 'offset' -// returns the new key of the passed in node and whether or not all the data in the reader -// has been consumed. -func (dm *DagModifier) modifyDag(n ipld.Node, offset uint64, data io.Reader) (*cid.Cid, bool, error) { +// modifyDag writes the data in 'dm.wrBuf' over the data in 'node' starting at 'offset' +// returns the new key of the passed in node. +func (dm *DagModifier) modifyDag(n ipld.Node, offset uint64) (*cid.Cid, error) { // If we've reached a leaf node. if len(n.Links()) == 0 { switch nd0 := n.(type) { case *mdag.ProtoNode: f, err := ft.FromBytes(nd0.Data()) if err != nil { - return nil, false, err + return nil, err } - n, err := data.Read(f.Data[offset:]) + _, err = dm.wrBuf.Read(f.Data[offset:]) if err != nil && err != io.EOF { - return nil, false, err + return nil, err } // Update newly written node.. b, err := proto.Marshal(f) if err != nil { - return nil, false, err + return nil, err } nd := new(mdag.ProtoNode) @@ -260,16 +259,10 @@ func (dm *DagModifier) modifyDag(n ipld.Node, offset uint64, data io.Reader) (*c nd.SetPrefix(&nd0.Prefix) err = dm.dagserv.Add(dm.ctx, nd) if err != nil { - return nil, false, err - } - - // Hey look! we're done! - var done bool - if n < len(f.Data[offset:]) { - done = true + return nil, err } - return nd.Cid(), done, nil + return nd.Cid(), nil case *mdag.RawNode: origData := nd0.RawData() bytes := make([]byte, len(origData)) @@ -278,9 +271,9 @@ func (dm *DagModifier) modifyDag(n ipld.Node, offset uint64, data io.Reader) (*c copy(bytes, origData[:offset]) // copy in new data - n, err := data.Read(bytes[offset:]) + n, err := dm.wrBuf.Read(bytes[offset:]) if err != nil && err != io.EOF { - return nil, false, err + return nil, err } // copy remaining data @@ -291,46 +284,39 @@ func (dm *DagModifier) modifyDag(n ipld.Node, offset uint64, data io.Reader) (*c nd, err := mdag.NewRawNodeWPrefix(bytes, nd0.Cid().Prefix()) if err != nil { - return nil, false, err + return nil, err } err = dm.dagserv.Add(dm.ctx, nd) if err != nil { - return nil, false, err - } - - // Hey look! we're done! - var done bool - if n < len(bytes[offset:]) { - done = true + return nil, err } - return nd.Cid(), done, nil + return nd.Cid(), nil } } node, ok := n.(*mdag.ProtoNode) if !ok { - return nil, false, ErrNotUnixfs + return nil, ErrNotUnixfs } f, err := ft.FromBytes(node.Data()) if err != nil { - return nil, false, err + return nil, err } var cur uint64 - var done bool for i, bs := range f.GetBlocksizes() { // We found the correct child to write into if cur+bs > offset { child, err := node.Links()[i].GetNode(dm.ctx, dm.dagserv) if err != nil { - return nil, false, err + return nil, err } - k, sdone, err := dm.modifyDag(child, offset-cur, data) + k, err := dm.modifyDag(child, offset-cur) if err != nil { - return nil, false, err + return nil, err } node.Links()[i].Cid = k @@ -338,12 +324,11 @@ func (dm *DagModifier) modifyDag(n ipld.Node, offset uint64, data io.Reader) (*c // Recache serialized node _, err = node.EncodeProtobuf(true) if err != nil { - return nil, false, err + return nil, err } - if sdone { + if dm.wrBuf.Len() == 0 { // No more bytes to write! - done = true break } offset = cur + bs @@ -352,7 +337,7 @@ func (dm *DagModifier) modifyDag(n ipld.Node, offset uint64, data io.Reader) (*c } err = dm.dagserv.Add(dm.ctx, node) - return node.Cid(), done, err + return node.Cid(), err } // appendData appends the blocks from the given chan to the end of this dag