Skip to content

Commit d807b60

Browse files
committed
LoRaMacSerializerData buffer overflow fix
A buffer size is computed and used to check the buffer size, but two flaws introduce the possibility of memory corruption: - the MIC field is not taken into account - the FOpts length is only taken into account when payloadsize>0, while the FOpts are always added With this patch the correct number of bytes is calculated. The calculation looks a bit verbose now, but for clarity it follows the exact order in which the message will be built up. Compiler optimization should combine all constants into one compound value.
1 parent 0d48900 commit d807b60

File tree

1 file changed

+11
-11
lines changed

1 file changed

+11
-11
lines changed

src/mac/LoRaMacSerializer.c

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -134,19 +134,19 @@ LoRaMacSerializerStatus_t LoRaMacSerializerData( LoRaMacMessageData_t* macMsg )
134134
+ LORAMAC_FHDR_F_CTRL_FIELD_SIZE
135135
+ LORAMAC_FHDR_F_CNT_FIELD_SIZE;
136136

137-
if( macMsg->FRMPayloadSize == 0 )
137+
computedBufSize += macMsg->FHDR.FCtrl.Bits.FOptsLen;
138+
139+
if( macMsg->FRMPayloadSize > 0 )
138140
{
139-
if( macMsg->BufSize < computedBufSize )
140-
{
141-
return LORAMAC_SERIALIZER_ERROR_BUF_SIZE;
142-
}
141+
computedBufSize += LORAMAC_F_PORT_FIELD_SIZE;
143142
}
144-
else
145-
{ //If FRMPayload >0, FPort field is present.
146-
if( macMsg->BufSize < computedBufSize + macMsg->FHDR.FCtrl.Bits.FOptsLen + macMsg->FRMPayloadSize + LORAMAC_F_PORT_FIELD_SIZE )
147-
{
148-
return LORAMAC_SERIALIZER_ERROR_BUF_SIZE;
149-
}
143+
144+
computedBufSize += macMsg->FRMPayloadSize;
145+
computedBufSize += LORAMAC_MIC_FIELD_SIZE;
146+
147+
if( macMsg->BufSize < computedBufSize )
148+
{
149+
return LORAMAC_SERIALIZER_ERROR_BUF_SIZE;
150150
}
151151

152152
macMsg->Buffer[bufItr++] = macMsg->MHDR.Value;

0 commit comments

Comments
 (0)