sysmocom
/
open5gs
forked from acouzens/open5gs
11
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

[SMF] Fix Gx/Gy assert() if more than 64 CCRs are sent 

The current code uses the cc request number as an index to the
transaction array (xact/xact_data). Since cc request number is a 32 bit
integer this is unfeasible for longer sessions and if more than a
handful of messages are exchanged per session.

The array size was already increased in #2038 which simply delays the
issue.
Furthermore, the current code asserts that cc_request_number is <=
MAX_CC_REQUEST_NUMBER which leads to an out-of-bounds write if
cc_request_number == MAX_CC_REQUEST_NUMBER.

Instead use a smaller array and index into it using cc_request_number
% array size. More than 2 requests should never be in flight at any one
time (initial or update request together with a termination request) so
an array size of 4 should be fine.
This commit is contained in:
Daniel Willmann 2023-06-02 16:07:00 +02:00 committed by Sukchan Lee
parent 93bcd7fda7
commit ef60207c1e
2 changed files with 29 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
src/smf/gx-path.c
View File

@ -28,9 +28,12 @@ struct sess_state {
os0_t peer_host; /* Peer Host */
#define MAX_CC_REQUEST_NUMBER 64
#define NUM_CC_REQUEST_SLOT 4
smf_sess_t *sess;
ogs_gtp_xact_t *xact[MAX_CC_REQUEST_NUMBER];
struct {
uint32_t cc_req_no;
ogs_gtp_xact_t *ptr;
} xact_data[NUM_CC_REQUEST_SLOT];
uint32_t cc_request_type;
uint32_t cc_request_number;
@ -103,6 +106,7 @@ void smf_gx_send_ccr(smf_sess_t *sess, ogs_gtp_xact_t *xact,
struct sockaddr_in sin;
struct sockaddr_in6 sin6;
uint32_t charging_id;
uint32_t req_slot;
ogs_assert(sess);
@ -191,11 +195,12 @@ void smf_gx_send_ccr(smf_sess_t *sess, ogs_gtp_xact_t *xact,
ogs_debug(" CC Request Type[%d] Number[%d]",
sess_data->cc_request_type, sess_data->cc_request_number);
ogs_assert(sess_data->cc_request_number <= MAX_CC_REQUEST_NUMBER);
/* Update session state */
sess_data->sess = sess;
sess_data->xact[sess_data->cc_request_number] = xact;
req_slot = sess_data->cc_request_number % NUM_CC_REQUEST_SLOT;
sess_data->xact_data[req_slot].ptr = xact;
sess_data->xact_data[req_slot].cc_req_no = sess_data->cc_request_number;
/* Set Origin-Host & Origin-Realm */
ret = fd_msg_add_origin(req, 0);
@ -713,7 +718,7 @@ static void smf_gx_cca_cb(void *data, struct msg **msg)
ogs_gtp_xact_t *xact = NULL;
smf_sess_t *sess = NULL;
ogs_diam_gx_message_t *gx_message = NULL;
uint32_t cc_request_number = 0;
uint32_t req_slot, cc_request_number = 0;
ogs_debug("[Credit-Control-Answer]");
@ -755,11 +760,13 @@ static void smf_gx_cca_cb(void *data, struct msg **msg)
ret = fd_msg_avp_hdr(avp, &hdr);
ogs_assert(ret == 0);
cc_request_number = hdr->avp_value->i32;
req_slot = cc_request_number % NUM_CC_REQUEST_SLOT;
ogs_debug(" CC-Request-Number[%d]", cc_request_number);
xact = sess_data->xact[cc_request_number];
xact = sess_data->xact_data[req_slot].ptr;
sess = sess_data->sess;
ogs_assert(sess_data->xact_data[req_slot].cc_req_no == cc_request_number);
ogs_assert(sess);
gx_message = ogs_calloc(1, sizeof(ogs_diam_gx_message_t));

28
src/smf/gy-path.c
View File

@ -29,12 +29,14 @@ struct sess_state {
os0_t peer_host; /* Peer Host */
#define MAX_CC_REQUEST_NUMBER 64
#define NUM_CC_REQUEST_SLOT 4
smf_sess_t *sess;
struct {
uint32_t cc_req_no;
bool pfcp;
void *ptr; /* INITIAL: ogs_gtp_xact_t, UPDATE: ogs_pfcp_xact_t */
} xact_data[MAX_CC_REQUEST_NUMBER];
} xact_data[NUM_CC_REQUEST_SLOT];
uint32_t cc_request_type;
uint32_t cc_request_number;
@ -567,7 +569,7 @@ void smf_gy_send_ccr(smf_sess_t *sess, void *xact,
struct session *session = NULL;
int new;
const char *service_context_id = "32251@3gpp.org";
uint32_t timestamp;
uint32_t timestamp, req_slot;
ogs_assert(xact);
ogs_assert(sess);
@ -657,15 +659,16 @@ void smf_gy_send_ccr(smf_sess_t *sess, void *xact,
ogs_debug(" CC Request Type[%d] Number[%d]",
sess_data->cc_request_type, sess_data->cc_request_number);
ogs_assert(sess_data->cc_request_number <= MAX_CC_REQUEST_NUMBER);
/* Update session state */
sess_data->sess = sess;
req_slot = sess_data->cc_request_number % NUM_CC_REQUEST_SLOT;
if (cc_request_type == OGS_DIAM_GY_CC_REQUEST_TYPE_UPDATE_REQUEST)
sess_data->xact_data[sess_data->cc_request_number].pfcp = true;
sess_data->xact_data[req_slot].pfcp = true;
else
sess_data->xact_data[sess_data->cc_request_number].pfcp = false;
sess_data->xact_data[sess_data->cc_request_number].ptr = xact;
sess_data->xact_data[req_slot].pfcp = false;
sess_data->xact_data[req_slot].cc_req_no = sess_data->cc_request_number;
sess_data->xact_data[req_slot].ptr = xact;
/* Origin-Host & Origin-Realm */
ret = fd_msg_add_origin(req, 0);
@ -902,7 +905,7 @@ static void smf_gy_cca_cb(void *data, struct msg **msg)
void *xact = NULL;
smf_sess_t *sess = NULL;
ogs_diam_gy_message_t *gy_message = NULL;
uint32_t cc_request_number = 0;
uint32_t req_slot, cc_request_number = 0;
ogs_debug("[Gy][Credit-Control-Answer]");
@ -944,11 +947,12 @@ static void smf_gy_cca_cb(void *data, struct msg **msg)
ret = fd_msg_avp_hdr(avp, &hdr);
ogs_assert(ret == 0);
cc_request_number = hdr->avp_value->i32;
req_slot = cc_request_number % NUM_CC_REQUEST_SLOT;
ogs_debug(" CC-Request-Number[%d]", cc_request_number);
xact = sess_data->xact_data[cc_request_number].ptr;
ogs_assert(xact);
xact = sess_data->xact_data[req_slot].ptr;
ogs_assert(sess_data->xact_data[req_slot].cc_req_no == cc_request_number);
sess = sess_data->sess;
ogs_assert(sess);
@ -1106,10 +1110,10 @@ out:
e->sess = sess;
e->gy_message = gy_message;
if (gy_message->cc_request_type == OGS_DIAM_GY_CC_REQUEST_TYPE_UPDATE_REQUEST) {
ogs_assert(sess_data->xact_data[sess_data->cc_request_number].pfcp == true);
ogs_assert(sess_data->xact_data[req_slot].pfcp == true);
e->pfcp_xact = xact;
} else {
ogs_assert(sess_data->xact_data[sess_data->cc_request_number].pfcp == false);
ogs_assert(sess_data->xact_data[req_slot].pfcp == false);
e->gtp_xact = xact;
}
rv = ogs_queue_push(ogs_app()->queue, e);