Serious memory leak bug?

[sksharma1501]

There is a serious bug in the file SGSNGGSN/GPRSL3Messages.cpp below: void GMMRoutingAreaIdIE::raLoad() { const char wMCC, wMNC; if (Sgsn::isUmts()) { wMCC = gConfig.getStr("UMTS.Identity.MCC").c_str(); wMNC = gConfig.getStr("UMTS.Identity.MNC").c_str(); mLAC = gConfig.getNum("UMTS.Identity.LAC"); } else { wMCC = gConfig.getStr("GSM.Identity.MCC").c_str(); wMNC = gConfig.getStr("GSM.Identity.MNC").c_str(); mLAC = gConfig.getNum("GSM.Identity.LAC"); } mRAC = gConfig.getNum("GPRS.RAC");

mMCC[0] = wMCC[0]-'0';
mMCC[1] = wMCC[1]-'0';
mMCC[2] = wMCC[2]-'0';
mMNC[0] = wMNC[0]-'0'; 
mMNC[1] = wMNC[1]-'0';
// 24.008 10.5.5.15 says if only two digits, MNC[2] is 0xf.
mMNC[2] = wMNC[2] ? (wMNC[2]-'0') : 0xf;

}

The problem here is that each call to c_str creates a char string in the same physical memory location. Thus wMCC value is overwritten by wMNC, which in turn is overwritten by mLAC (internal to getNum) and then mRAC (internal to getNum). By the time mMCC and mMNC digits are getting assigned, they map to a string "GPRS.RAC". Hence these digits are always wrong.

There are at least two consequences of this: a) Each time an RRC CONNECTION REQUEST comes in, OpenBTS-UMTS tries to match up the fields of p-TMSI, MCC, MNC, LAC, RAC. But since the MCC/MNC digits received in last ATTACH ACCEPT were wrong, they don't match up with the configured value (default: 001/01). This leads to a new UE context creation each time inside handleRrcConnectionRequest, as below. Previous UE context is never deleted, causing memory leaks.

AsnUeId aid(msg->initialUE_Identity);

const char *comment = "UL_CCCH_MessageType_PR_rrcConnectionRequest";
UEInfo *uep = gRrc.findUeByAsnId(&aid);
if (uep == NULL) {
    uep = new UEInfo(&aid);
    comment = "UL_CCCH_MessageType_PR_rrcConnectionRequest (new UE)";
}

b) If same UE does a ROUTING AREA UPDATE sometime after ATTACH COMPLETE (and SIGNALLING CONNECTION RELEASE INDICATION), it is still misunderstood as a new UE. Then OpenBTS-UMTS thinks the UE has not attached before, and sends ROUTING AREA UPDATE REJECT message instead.

The following change should fix this bug:

void GMMRoutingAreaIdIE::raLoad() { const char wMCC, wMNC; if (Sgsn::isUmts()) { wMCC = gConfig.getStr("UMTS.Identity.MCC").c_str(); mMCC[0] = wMCC[0]-'0'; mMCC[1] = wMCC[1]-'0'; mMCC[2] = wMCC[2]-'0'; wMNC = gConfig.getStr("UMTS.Identity.MNC").c_str(); mMNC[0] = wMNC[0]-'0'; mMNC[1] = wMNC[1]-'0'; // 24.008 10.5.5.15 says if only two digits, MNC[2] is 0xf. mMNC[2] = wMNC[2] ? (wMNC[2]-'0') : 0xf; mLAC = gConfig.getNum("UMTS.Identity.LAC"); } else { wMCC = gConfig.getStr("GSM.Identity.MCC").c_str(); mMCC[0] = wMCC[0]-'0'; mMCC[1] = wMCC[1]-'0'; mMCC[2] = wMCC[2]-'0'; wMNC = gConfig.getStr("GSM.Identity.MNC").c_str(); mMNC[0] = wMNC[0]-'0'; mMNC[1] = wMNC[1]-'0'; // 24.008 10.5.5.15 says if only two digits, MNC[2] is 0xf. mMNC[2] = wMNC[2] ? (wMNC[2]-'0') : 0xf; mLAC = gConfig.getNum("GSM.Identity.LAC"); } mRAC = gConfig.getNum("GPRS.RAC"); }

[alejandro-amo]

ey @sksharma1501 buddy, how is it going? I'm doing a mix-and-match of all the good patches and enhancements for openbts-UMTS project in a new, mantained, open fork so we all can better control the code and submit changes as well as mutually benefit from the other's ideas and knowledge.

at the time of this writing I have merged three interesting forks with patches and I have modified the code in order to correctly run with newest USRP UHD drivers.

I will like you to push your ideas there. https://github.com/EurecatSecurity/OpenBTS-UMTS