buffer overflow with fixedlength2 + header/footer

[paulstadler]

We've found using a header/footer construct in fixedlength2 causes a buffer overflow within go-corelib.

To reproduce, construct an input such as:
<REC_HDR>
<Lots of lines here...
...
...
...
<REC_FTR>

Tracing into go-corelib it looks like there's a default buffer size of 4096, so the input record needs to be greater than that to reproduce.

It looks ilke reader.lineBuf[i] is getting set to a pointer inside the buffer. When the buffer wraps, the lineBuf gets corrupted.

See attached files to reproduce. This occurs (for these data), when r.linesRead is 118 (or shortly thereafter).

I think something like this could fix it, but not sure if it's the optimal solution (this works though...).

This is in: fileformat/flatfile/fixedlength/reader.go

func (r *reader) readLine() error {
	for {
		// note1: ios.ByteReadLine returns a ln with trailing '\n' (and/or '\r') dropped.
		// note2: ios.ByteReadLine won't return io.EOF if ln returned isn't empty.
		b, err := ios.ByteReadLine(r.r)
		switch {
		case err == io.EOF:
			return io.EOF
		case err != nil:
			return ErrInvalidFixedLength(r.fmtErrStr(r.linesRead+1, err.Error()))
		}
		r.linesRead++
		if len(b) > 0 {
			// Copy data out of the buffer into our local line buffer
			lineCopy := make([]byte, len(b))
			_ = copy(lineCopy, b)
			r.linesBuf = append(r.linesBuf, line{lineNum: r.linesRead, b: lineCopy})
			// This causes buffer overruns to appear in local data:
			// r.linesBuf = append(r.linesBuf, line{lineNum: r.linesRead, b: b})
			return nil
		}
	}
}

Cheers,
Paul

schema.txt
data.txt

[jf-tech]

@paulstadler ack. Will take a look early next week.

[jf-tech]

@paulstadler the fix has been posted at #214. Do you mind sync the branch https://github.com/jf-tech/omniparser/tree/213 and give it a test to confirm it fixes your issue, before I merge the fix to master?

Also another question: once merged into master, do you mind using master for a few days / max 1 week or two. We're adding a new feature to the omniparser and would like to cut an official release once that's in, instead of too many tiny releases. What do you think?

Many thanks.

[paulstadler]

Absolutely! I'll test here today.
Cheers,
Paul

[paulstadler]

Confirmed working here and reviewed code. Looks good. Thanks!

[jf-tech]

@paulstadler Merged into master. Like I mentioned earlier, please use master @ latest for a short while - before we finish a new EDI feature and cut a release.

Also please consider sponsoring the project! Appreciate it!