pdu: add check when unpacking MAC PDUs to not read beyond PDU length

we've checked the same when unpacking the subheaders but missed the
case where the payload was read beyond the PDU length, as has been
seen with a malformed RAR PDU.
This commit is contained in:
Andre Puschmann 2021-01-11 10:39:09 +01:00
parent 10da7df194
commit fcf481b83e
2 changed files with 26 additions and 9 deletions

View File

@ -218,7 +218,7 @@ public:
void add_sdu(uint32_t sdu_sz) { buffer_tx->N_bytes += sdu_sz; }
// Section 6.1.2
void parse_packet(uint8_t* ptr)
uint32_t parse_packet(uint8_t* ptr)
{
uint8_t* init_ptr = ptr;
nof_subheaders = 0;
@ -230,19 +230,35 @@ public:
}
if (ret && ((ptr - init_ptr) >= (int32_t)pdu_len)) {
// stop processing last subheader indicates another one but all bytes are consume
// stop processing last subheader indicates another one but all bytes are consumed
nof_subheaders = 0;
INFO("Corrupted MAC PDU - all bytes have been consumed (pdu_len=%d)\n", pdu_len);
if (log_h) {
log_h->info_hex(
log_h->warning_hex(
init_ptr, pdu_len, "Corrupted MAC PDU - all bytes have been consumed (pdu_len=%d)\n", pdu_len);
} else {
srslte::console("Corrupted MAC PDU - all bytes have been consumed (pdu_len=%d)\n", pdu_len);
}
return SRSLTE_ERROR;
}
} while (ret && (nof_subheaders + 1) < (int)max_subheaders && ((int32_t)pdu_len > (ptr - init_ptr)));
for (int i = 0; i < nof_subheaders; i++) {
subheaders[i].read_payload(&ptr);
// stop processing if we read payload beyond the PDU length
if ((ptr - init_ptr) > (int32_t)pdu_len) {
nof_subheaders = 0;
if (log_h) {
log_h->warning_hex(
init_ptr, pdu_len, "Corrupted MAC PDU - all bytes have been consumed (pdu_len=%d)\n", pdu_len);
} else {
srslte::console("Corrupted MAC PDU - all bytes have been consumed (pdu_len=%d)\n", pdu_len);
}
return SRSLTE_ERROR;
}
}
return SRSLTE_SUCCESS;
}
protected:

View File

@ -218,9 +218,10 @@ void sch_pdu::parse_packet(uint8_t* ptr)
if (n_sub >= 0) {
subheaders[nof_subheaders - 1].set_payload_size(n_sub);
} else {
ERROR("Corrupted MAC PDU (read_len=%d, pdu_len=%d)\n", read_len, pdu_len);
if (log_h) {
log_h->info_hex(ptr, pdu_len, "Corrupted MAC PDU (read_len=%d, pdu_len=%d)\n", read_len, pdu_len);
log_h->warning_hex(ptr, pdu_len, "Corrupted MAC PDU (read_len=%d, pdu_len=%d)\n", read_len, pdu_len);
} else {
srslte::console("Corrupted MAC PDU (read_len=%d, pdu_len=%d)\n", read_len, pdu_len);
}
// reset PDU
@ -1017,7 +1018,7 @@ uint8_t sch_subh::phr_report_table(float phr_value)
std::string rar_pdu::to_string()
{
std::string msg("MAC PDU for RAR. ");
std::string msg("MAC PDU for RAR: ");
msg += pdu::to_string();
return msg;
}
@ -1072,7 +1073,7 @@ bool rar_pdu::write_packet(uint8_t* ptr)
int32_t pad_len = rem_len - payload_len;
if (pad_len < 0) {
if (log_h) {
log_h->error("Error packing RAR PDU (payload_len=%d, rem_len=%d)\n", payload_len, rem_len);
log_h->warning("Error packing RAR PDU (payload_len=%d, rem_len=%d)\n", payload_len, rem_len);
} else {
srslte::console("Error packing RAR PDU (payload_len=%d, rem_len=%d)\n", payload_len, rem_len);
}
@ -1095,7 +1096,7 @@ std::string rar_subh::to_string()
char tmp[16];
srslte_vec_sprint_hex(tmp, sizeof(tmp), grant, RAR_GRANT_LEN);
ss << tmp << "\n";
ss << tmp;
return ss.str();
}