CVE-2020-28349: Improper Input Validation (CWE-20)
value: An inaccurate frame deduplication process in ChirpStack Network Server 3.9.0 allows a malicious gateway to perform uplink Denial of Service via malformed frequency attributes in CollectAndCallOnceCollect in internal/uplink/collect.go. NOTE: the vendor's position is that there are no "guarantees that allowing untrusted LoRa gateways to the network should still result in a secure network.
CWE description: The product receives input or data, but it does
        not validate or incorrectly validates that the input has the
        properties that are required to process the data safely and
        correctly.
CVSS v3 base score: 6.5
Published: 2020-11-09T01:15Z
Repository: https://github.com/brocaar/chirpstack-network-server
Language: Go
Files changed: internal/gateway/rx_info.go, internal/gateway/stats/stats.go, internal/uplink/uplink.go
Commit date: 2020-07-09 08:45:02 +0200
Commit message: Improve error handling of unknown gateways.

Diff: diff --git a/internal/gateway/rx_info.go b/internal/gateway/rx_info.go
index 35b7e40..bef08f5 100644
--- a/internal/gateway/rx_info.go
+++ b/internal/gateway/rx_info.go
@@ -23,72 +23,57 @@ import (
 // UpdateMetaDataInRxInfoSet updates the gateway meta-data in the
 // given rx-info set. It will:
 //   - add the gateway location
 //   - set the FPGA id if available
 //   - decrypt the fine-timestamp (if available and AES key is set)
-func UpdateMetaDataInRxInfoSet(ctx context.Context, db sqlx.Queryer, rxInfo []*gw.UplinkRXInfo) error {
-	for i := range rxInfo {
-		id := helpers.GetGatewayID(rxInfo[i])
+func UpdateMetaDataInRxInfoSet(ctx context.Context, db sqlx.Queryer, rxInfoSet []*gw.UplinkRXInfo) []*gw.UplinkRXInfo {
+	var out []*gw.UplinkRXInfo
+
+	for i := range rxInfoSet {
+		rxInfo := rxInfoSet[i]
+
+		id := helpers.GetGatewayID(rxInfo)
 		g, err := storage.GetAndCacheGateway(ctx, db, id)
 		if err != nil {
-			log.WithFields(log.Fields{
-				"ctx_id":     ctx.Value(logging.ContextIDKey),
-				"gateway_id": id,
-			}).WithError(err).Error("get gateway error")
+			if errors.Cause(err) == storage.ErrDoesNotExist {
+				log.WithFields(log.Fields{
+					"ctx_id":     ctx.Value(logging.ContextIDKey),
+					"gateway_id": id,
+				}).Warning("uplink received by unknown gateway")
+			} else {
+				log.WithFields(log.Fields{
+					"ctx_id":     ctx.Value(logging.ContextIDKey),
+					"gateway_id": id,
+				}).WithError(err).Error("get gateway error")
+			}
 			continue
 		}
 
 		// set gateway location
-		rxInfo[i].Location = &common.Location{
+		rxInfo.Location = &common.Location{
 			Latitude:  g.Location.Latitude,
 			Longitude: g.Location.Longitude,
 			Altitude:  g.Altitude,
 		}
 
 		var board storage.GatewayBoard
-		if int(rxInfo[i].Board) < len(g.Boards) {
-			board = g.Boards[int(rxInfo[i].Board)]
+		if int(rxInfo.Board) < len(g.Boards) {
+			board = g.Boards[int(rxInfo.Board)]
 		}
 
 		// set FPGA ID
 		// this is useful when the AES decryption key is not set as it
 		// indicates which key to use for decryption
-		if rxInfo[i].FineTimestampType == gw.FineTimestampType_ENCRYPTED && board.FPGAID != nil {
-			tsInfo := rxInfo[i].GetEncryptedFineTimestamp()
-			if tsInfo == nil {
-				log.WithFields(log.Fields{
-					"ctx_id":     ctx.Value(logging.ContextIDKey),
-					"gateway_id": id,
-				}).Error("encrypted_fine_timestamp must not be nil")
-				continue
-			}
-
+		if tsInfo := rxInfo.GetEncryptedFineTimestamp(); tsInfo != nil && board.FPGAID != nil {
 			if len(tsInfo.FpgaId) == 0 {
 				tsInfo.FpgaId = board.FPGAID[:]
 			}
 		}
 
 		// decrypt fine-timestamp when the AES key is known
-		if rxInfo[i].FineTimestampType == gw.FineTimestampType_ENCRYPTED && board.FineTimestampKey != nil {
-			tsInfo := rxInfo[i].GetEncryptedFineTimestamp()
-			if tsInfo == nil {
-				log.WithFields(log.Fields{
-					"ctx_id":     ctx.Value(logging.ContextIDKey),
-					"gateway_id": id,
-				}).Error("encrypted_fine_timestamp must not be nil")
-				continue
-			}
-
-			if rxInfo[i].Time == nil {
-				log.WithFields(log.Fields{
-					"ctx_id":     ctx.Value(logging.ContextIDKey),
-					"gateway_id": id,
-				}).Error("time must not be nil")
-				continue
-			}
-
-			rxTime, err := ptypes.Timestamp(rxInfo[i].Time)
+		if tsInfo := rxInfo.GetEncryptedFineTimestamp(); tsInfo != nil && board.FineTimestampKey != nil {
+			rxTime, err := ptypes.Timestamp(rxInfo.GetTime())
 			if err != nil {
 				log.WithFields(log.Fields{
 					"ctx_id":     ctx.Value(logging.ContextIDKey),
 					"gateway_id": id,
 				}).WithError(err).Error("get timestamp error")
@@ -101,18 +86,20 @@ func UpdateMetaDataInRxInfoSet(ctx context.Context, db sqlx.Queryer, rxInfo []*g
 					"gateway_id": id,
 				}).WithError(err).Error("decrypt fine-timestamp error")
 				continue
 			}
 
-			rxInfo[i].FineTimestampType = gw.FineTimestampType_PLAIN
-			rxInfo[i].FineTimestamp = &gw.UplinkRXInfo_PlainFineTimestamp{
+			rxInfo.FineTimestampType = gw.FineTimestampType_PLAIN
+			rxInfo.FineTimestamp = &gw.UplinkRXInfo_PlainFineTimestamp{
 				PlainFineTimestamp: &plainTS,
 			}
 		}
+
+		out = append(out, rxInfo)
 	}
 
-	return nil
+	return out
 }
 
 func decryptFineTimestamp(key lorawan.AES128Key, rxTime time.Time, ts gw.EncryptedFineTimestamp) (gw.PlainFineTimestamp, error) {
 	var plainTS gw.PlainFineTimestamp
 
diff --git a/internal/gateway/stats/stats.go b/internal/gateway/stats/stats.go
index 0e08503..a63df11 100644
--- a/internal/gateway/stats/stats.go
+++ b/internal/gateway/stats/stats.go
@@ -16,10 +16,12 @@ import (
 	"github.com/brocaar/chirpstack-network-server/internal/logging"
 	"github.com/brocaar/chirpstack-network-server/internal/storage"
 	loraband "github.com/brocaar/lorawan/band"
 )
 
+var ErrAbort = errors.New("abort")
+
 type statsContext struct {
 	ctx          context.Context
 	gateway      storage.Gateway
 	gatewayStats gw.GatewayStats
 }
@@ -38,10 +40,13 @@ func Handle(ctx context.Context, stats gw.GatewayStats) error {
 		gatewayStats: stats,
 	}
 
 	for _, t := range tasks {
 		if err := t(&sctx); err != nil {
+			if err == ErrAbort {
+				return nil
+			}
 			return err
 		}
 	}
 
 	return nil
@@ -49,11 +54,19 @@ func Handle(ctx context.Context, stats gw.GatewayStats) error {
 
 func getGateway(ctx *statsContext) error {
 	gatewayID := helpers.GetGatewayID(&ctx.gatewayStats)
 	gw, err := storage.GetAndCacheGateway(ctx.ctx, storage.DB(), gatewayID)
 	if err != nil {
-		return errors.Wrap(err, "get gateway error")
+		if errors.Cause(err) == storage.ErrDoesNotExist {
+			log.WithFields(log.Fields{
+				"ctx_id":     ctx.ctx.Value(logging.ContextIDKey),
+				"gateway_id": gatewayID,
+			}).Warning("gateway/stats: stats received by unknown gateway")
+			return ErrAbort
+		} else {
+			return errors.Wrap(err, "get gateway error")
+		}
 	}
 
 	ctx.gateway = gw
 	return nil
 }
diff --git a/internal/uplink/uplink.go b/internal/uplink/uplink.go
index 38fe052..65e558c 100644
--- a/internal/uplink/uplink.go
+++ b/internal/uplink/uplink.go
@@ -176,10 +176,18 @@ func collectUplinkFrames(ctx context.Context, uplinkFrame gw.UplinkFrame) error
 		return err
 	})
 }
 
 func handleCollectedUplink(ctx context.Context, uplinkFrame gw.UplinkFrame, rxPacket models.RXPacket) error {
+	// update the gateway meta-data
+	rxPacket.RXInfoSet = gateway.UpdateMetaDataInRxInfoSet(ctx, storage.DB(), rxPacket.RXInfoSet)
+
+	// Return if the RXInfoSet is empty.
+	if len(rxPacket.RXInfoSet) == 0 {
+		return nil
+	}
+
 	var uplinkIDs []uuid.UUID
 	for _, p := range rxPacket.RXInfoSet {
 		uplinkIDs = append(uplinkIDs, helpers.GetUplinkID(p))
 	}
 
@@ -187,15 +195,10 @@ func handleCollectedUplink(ctx context.Context, uplinkFrame gw.UplinkFrame, rxPa
 		"uplink_ids": uplinkIDs,
 		"mtype":      rxPacket.PHYPayload.MHDR.MType,
 		"ctx_id":     ctx.Value(logging.ContextIDKey),
 	}).Info("uplink: frame(s) collected")
 
-	// update the gateway meta-data
-	if err := gateway.UpdateMetaDataInRxInfoSet(ctx, storage.DB(), rxPacket.RXInfoSet); err != nil {
-		log.WithError(err).Error("uplink: update gateway meta-data in rx-info set error")
-	}
-
 	// log the frame for each receiving gateway.
 	if err := framelog.LogUplinkFrameForGateways(ctx, ns.UplinkFrameLog{
 		PhyPayload: uplinkFrame.PhyPayload,
 		TxInfo:     rxPacket.TXInfo,
 		RxInfo:     rxPacket.RXInfoSet,

Vulnerable code: func UpdateMetaDataInRxInfoSet(ctx context.Context, db sqlx.Queryer, rxInfo []*gw.UplinkRXInfo) error {
	for i := range rxInfo {
		id := helpers.GetGatewayID(rxInfo[i])
			log.WithFields(log.Fields{
				"ctx_id":     ctx.Value(logging.ContextIDKey),
				"gateway_id": id,
			}).WithError(err).Error("get gateway error")
		rxInfo[i].Location = &common.Location{
		if int(rxInfo[i].Board) < len(g.Boards) {
			board = g.Boards[int(rxInfo[i].Board)]
		if rxInfo[i].FineTimestampType == gw.FineTimestampType_ENCRYPTED && board.FPGAID != nil {
			tsInfo := rxInfo[i].GetEncryptedFineTimestamp()
			if tsInfo == nil {
				log.WithFields(log.Fields{
					"ctx_id":     ctx.Value(logging.ContextIDKey),
					"gateway_id": id,
				}).Error("encrypted_fine_timestamp must not be nil")
				continue
			}

		if rxInfo[i].FineTimestampType == gw.FineTimestampType_ENCRYPTED && board.FineTimestampKey != nil {
			tsInfo := rxInfo[i].GetEncryptedFineTimestamp()
			if tsInfo == nil {
				log.WithFields(log.Fields{
					"ctx_id":     ctx.Value(logging.ContextIDKey),
					"gateway_id": id,
				}).Error("encrypted_fine_timestamp must not be nil")
				continue
			}

			if rxInfo[i].Time == nil {
				log.WithFields(log.Fields{
					"ctx_id":     ctx.Value(logging.ContextIDKey),
					"gateway_id": id,
				}).Error("time must not be nil")
				continue
			}

			rxTime, err := ptypes.Timestamp(rxInfo[i].Time)
			rxInfo[i].FineTimestampType = gw.FineTimestampType_PLAIN
			rxInfo[i].FineTimestamp = &gw.UplinkRXInfo_PlainFineTimestamp{
	return nil
		return errors.Wrap(err, "get gateway error")
	// update the gateway meta-data
	if err := gateway.UpdateMetaDataInRxInfoSet(ctx, storage.DB(), rxPacket.RXInfoSet); err != nil {
		log.WithError(err).Error("uplink: update gateway meta-data in rx-info set error")
	}

Fixed code: func UpdateMetaDataInRxInfoSet(ctx context.Context, db sqlx.Queryer, rxInfoSet []*gw.UplinkRXInfo) []*gw.UplinkRXInfo {
	var out []*gw.UplinkRXInfo

	for i := range rxInfoSet {
		rxInfo := rxInfoSet[i]

		id := helpers.GetGatewayID(rxInfo)
			if errors.Cause(err) == storage.ErrDoesNotExist {
				log.WithFields(log.Fields{
					"ctx_id":     ctx.Value(logging.ContextIDKey),
					"gateway_id": id,
				}).Warning("uplink received by unknown gateway")
			} else {
				log.WithFields(log.Fields{
					"ctx_id":     ctx.Value(logging.ContextIDKey),
					"gateway_id": id,
				}).WithError(err).Error("get gateway error")
			}
		rxInfo.Location = &common.Location{
		if int(rxInfo.Board) < len(g.Boards) {
			board = g.Boards[int(rxInfo.Board)]
		if tsInfo := rxInfo.GetEncryptedFineTimestamp(); tsInfo != nil && board.FPGAID != nil {
		if tsInfo := rxInfo.GetEncryptedFineTimestamp(); tsInfo != nil && board.FineTimestampKey != nil {
			rxTime, err := ptypes.Timestamp(rxInfo.GetTime())
			rxInfo.FineTimestampType = gw.FineTimestampType_PLAIN
			rxInfo.FineTimestamp = &gw.UplinkRXInfo_PlainFineTimestamp{

		out = append(out, rxInfo)
	return out
var ErrAbort = errors.New("abort")

			if err == ErrAbort {
				return nil
			}
		if errors.Cause(err) == storage.ErrDoesNotExist {
			log.WithFields(log.Fields{
				"ctx_id":     ctx.ctx.Value(logging.ContextIDKey),
				"gateway_id": gatewayID,
			}).Warning("gateway/stats: stats received by unknown gateway")
			return ErrAbort
		} else {
			return errors.Wrap(err, "get gateway error")
		}
	// update the gateway meta-data
	rxPacket.RXInfoSet = gateway.UpdateMetaDataInRxInfoSet(ctx, storage.DB(), rxPacket.RXInfoSet)

	// Return if the RXInfoSet is empty.
	if len(rxPacket.RXInfoSet) == 0 {
		return nil
	}