Skip to content

lisafs: bound Walk and WalkStat response payloads to maxMessageSize#13243

Open
ibondarenko1 wants to merge 1 commit into
google:masterfrom
ibondarenko1:hardening/lisafs-walk-payload-bound
Open

lisafs: bound Walk and WalkStat response payloads to maxMessageSize#13243
ibondarenko1 wants to merge 1 commit into
google:masterfrom
ibondarenko1:hardening/lisafs-walk-payload-bound

Conversation

@ibondarenko1
Copy link
Copy Markdown

Problem

WalkHandler and WalkStatHandler size the response payload buffer from the request-controlled req.Path length, and only reject it against math.MaxUint32:

maxPayloadSize := respMetaSize + (len(req.Path) * (*Inode)(nil).SizeBytes())
if maxPayloadSize > math.MaxUint32 {
	// Too much to walk, can't do.
	return 0, unix.EIO
}
payloadBuf := comm.PayloadBuf(uint32(maxPayloadSize))

comm.PayloadBuf for the channel transport returns a slice into a window of maxMessageSize bytes. A maxPayloadSize between maxMessageSize and MaxUint32 therefore indexes past that window.

Sibling precedent

PReadHandler, in the same file, bounds its response payload correctly:

respPayloadLen := respMetaSize + req.Count
if respPayloadLen > c.maxMessageSize {
	return 0, unix.ENOBUFS
}
payloadBuf := comm.PayloadBuf(respPayloadLen)

Change

Bound maxPayloadSize in both Walk handlers against c.maxMessageSize, matching PReadHandler. math is no longer referenced in the file, so its import is dropped. Two one-line changes plus the import removal.

Scope

Hardening. A handler panic of this shape is recovered by Connection.handleMsg and turned into an EREMOTEIO response, so the gofer does not crash, and the request is only reachable from a compromised sentry. This change removes the reliance on that recover() net and makes the two Walk handlers consistent with PReadHandler.

Getdents64Handler has a related but structurally different unbounded re-allocation; it is left for a separate change because its fix is not a one-line bound and would not belong in this minimal cleanup.

WalkHandler and WalkStatHandler size the response payload buffer from
the request-controlled path length and only reject it when it exceeds
math.MaxUint32 (4 GiB). comm.PayloadBuf slices into a window of
maxMessageSize bytes, so any size between maxMessageSize and MaxUint32
indexes past that window.

The sibling PReadHandler bounds its response payload against
c.maxMessageSize. Apply the same bound to both Walk handlers. math is no
longer referenced in the file, so its import is dropped.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants