Question about the router: bound-checking for hop fields possibly missing

[jcp19]

Hi everyone,

First, I apologize if this report/question is a bit too hand-wavy at times, but while proving the correctness of the router in the context of VerifiedSCION, I noticed that something looks weird.

More concretely, in file pkg/slayers/path/scion/decoded.go, there are the following definitions, together with comments expressing a few assumptions about the structure of the scion packets:

const (
	// MaxINFs is the maximum number of info fields in a SCION path.
	MaxINFs = 3
	// MaxHops is the maximum number of hop fields in a SCION path.
	MaxHops = 64
)

While the restriction on MaxINFs looks OK, the restriction that we have at most 64 hop fields seems to not be enforced: when we decode a Base, we store in the field NumHops the sum of the lengths of all segments. The length of each segment cannot exceed 64, but their sums can. As far as I can tell, there is no check performed while forwarding a packet that NumINFs is less than 64. While processing a SCION packet, we eventually call IncPath, which has the effect of incrementing the field CurrHF in PathMeta. As far as I can tell, nothing so far prohibits this field from getting to value 64 (if its original value was 63). The problem here is that, when we serialize the output packet (including its packet), if its value is at least 64, we will only serialize its 6 least significant bits, instead of storing its full value (the operation that serializes the CurrHF is uint32(m.CurrHF&0x3F)<<24). As such, I believe that, as is, the router would allow sending packets whose paths are too long until a point when they become invalid and are dropped. Is my understanding correct?

[jiceatscion]

Due to the headers definitions, a scion packet with more than 64 hop fields cannot actually exist. It is just a packet with 64 hop fields plus stuff that looks like hop fields at the beginning of the payload. I guess this means that we're missing a check for the sum of segment lengths, since that's the only thing that indicates the packet is broken before actually reaching the end of the path. I'll try and produce this and fix it if needed.

[jiceatscion]

Hem... I take that back. It is possible to fit up to 80 hops fields in a maximum header. So, may be more checks required.