Description

The rawData type in sdk/app.go is used to hold receipt, storage, and transaction data. There are two fields, a list ordered and a map special. The idea is that generally, data will be added to ordered, but in order to be able to pin certain data points to a specific index, they may be added to special instead. When converting a rawData to a list, the data in special will occur at their respective index, and the remaining unclaimed entries will be filled by the entries of ordered, in order.

The add function used to add data to a rawData is implemented as follows:

func (q *rawData[T]) add(data T, index ...int) {
	if len(index) > 1 {
		panic("no more than one index should be supplied")
	}
	if q.special == nil {
		q.special = make(map[int]T)
	}
	if len(index) == 1 {
		q.special[index[0]] = data
	} else {
		q.ordered = append(q.ordered, data)
	}
}

If an index is given, then data is inserted into the map at that index. However, it is not checked whether a value at that key might already exist. Thus, it can happen that a previous entry is overwritten.

Impact

Entries of q.special may be overwritten by add. The function add is an internal function, used by the public wrappers AddReceipt, AddStorage, and AddTransaction. The intention of the special map is to pin receipts, storage, and transaction data to a specific index. Because of this, reusing the same index would be a mistake.

Currently this mistake will not be caught. It could thus happen that a user of the sdk intends to pin two data points A and B but accidentally uses the same index. Only the last added of the two will then be available at that index (say B), but in circuit they will use this single data point for the use case of both A and B.

It appears likely that such mistakes would be detected during proper testing.

Recommendations

We recommend to check in add whether q.special already has a value at key index[0]. If this is the case, the function should panic with an error message warning about index reuse.

Remediation

This issue has been acknowledged by Brevis, and a fix was implemented in commit 7b2df59a↗.