[reSIProcate] Probable memory leaks in DialogUsageManager::incomingProcess
Byron Campen
bcampen at estacado.net
Wed Mar 7 16:18:38 CST 2007
Well, I haven't seen any activity in this thread, but here's a
patch. I'm going to apply it tomorrow unless I get some feedback
telling me to do otherwise.

Best regards,
Byron Campen
> Just after running incoming messages through the feature chains,
> we enter a try block whose catch logs an error, and takes no
> further action.
>
> *snip*
> catch(BaseException& e)
> {
> //unparseable, bad 403 w/ 2543 trans it from FWD, etc
> ErrLog(<<"Illegal message rejected: " << e.getMessage());
> }
> *snip*
>
> Looking at the code, incomingProcess is ultimately responsible for
> making and sending responses to requests. It may delegate this
> responsibility, but its job is to ensure that _everything_ gets a
> reply of some sort before returning. Failing to reply to a request
> will cause the stack to leak memory, as I have said a million
> times. I can see several places where an exception will be thrown
> upon receiving garbage from the wire.
>
> ======================================================================
> ==========
>
> in MasterProfile::getUnsupportedOptionsTags() at line 110 called by
> DialogUsageManager::validateRequiredOptions() at line 1325 called by
> DialogUsageManager::incomingProcess() at line 1218
> *snip*
> Tokens
> MasterProfile::getUnsupportedOptionsTags(const Tokens&
> requiresOptionTags)
> {
> Tokens tokens;
> for (Tokens::const_iterator i=requiresOptionTags.begin(); i !=
> requiresOptionTags.end(); ++i)
> {
> // if this option is not supported
> if (!mSupportedOptionTags.find(*i))
> {
> tokens.push_back(*i);
> }
> }
>
> return tokens;
> }
> *snip*
>
> The code highlighted will throw if we encounter a malformed Token
> (a Requires header-field (Token::parse() won't throw on us itself,
> but it calls parseParameters(), which means all bets are off)
>
> ======================================================================
> ==========
>
> in DialogUsageManager::validateContent() at line 1347 called by
> DialogUsageManager::incomingProcess() at line 1223
> *snip*
> // RFC3261 - 8.2.3
> // Don't need to validate content headers if they are specified
> as optional in the content-disposition
> if (!(request.exists(h_ContentDisposition) &&
> request.header(h_ContentDisposition).exists(p_handling) &&
> isEqualNoCase(request.header(h_ContentDisposition).param
> (p_handling), Symbols::Optional)))
> {
> *snip*
>
> The code highlighted will throw if Content-Disposition is malformed.
>
> ======================================================================
> ==========
>
> in MasterProfile::isMimeTypeSupported() at line 143 called by
> DialogUsageManager::validateContent() at line 1351 called by
> DialogUsageManager::incomingProcess() at line 1223
>
> *snip*
> bool
> MasterProfile::isMimeTypeSupported(const MethodTypes& method, const
> Mime& mimeType)
> {
> std::map<MethodTypes, Mimes>::iterator found =
> mSupportedMimeTypes.find(method);
> if (found != mSupportedMimeTypes.end())
> {
> return found->second.find(mimeType);
> }
> return false;
> }
> *snip*
>
> The code highlighted will throw if the mimeType passed (the Content-
> Type header-field-value) is malformed.
>
> ======================================================================
> ==========
>
> in MasterProfile::isContentEncodingSupported() at line 184 called by
> DialogUsageManager::validateContent() at line 1363 called by
> DialogUsageManager::incomingProcess() at line 1223
> *snip*
> bool
> MasterProfile::isContentEncodingSupported(const Token& encoding) const
> {
> return mSupportedEncodings.find(encoding);
> }
> *snip*
>
> The highlighted text will throw if the Token (the Content-Encoding
> header-field-value) is malformed.
>
> ======================================================================
> ==========
>
> in MasterProfile::isLanguageSupported() at line 184 called by
> DialogUsageManager::validateContent() at line 1375 called by
> DialogUsageManager::incomingProcess() at line 1223
> *snip*
> bool
> MasterProfile::isLanguageSupported(const Tokens& langs) const
> {
> for (Tokens::const_iterator i=langs.begin(); i != langs.end(); ++i)
> {
> if (mSupportedLanguages.find(*i) == false)
> {
> return false;
> }
> }
> return true;
> }
> *snip*
>
> ======================================================================
> ==========
>
> in MasterProfile::isMimeTypeSupported() at line 143 called by
> DialogUsageManager::validateAccept() at line 1401 called by
> DialogUsageManager::incomingProcess() at line 1228
> *snip*
> bool
> MasterProfile::isMimeTypeSupported(const MethodTypes& method, const
> Mime& mimeType)
> {
> std::map<MethodTypes, Mimes>::iterator found =
> mSupportedMimeTypes.find(method);
> if (found != mSupportedMimeTypes.end())
> {
> return found->second.find(mimeType);
> }
> return false;
> }
> *snip*
>
> The code highlighted will throw if the mimeType passed (an Accept
> header-field-value) is malformed.
>
> ======================================================================
> ==========
>
> in DialogUsageManager::incomingProcess() at line 1234
> *snip*
> }
> }
> if (sipMsg->header(h_From).exists(p_tag))
> {
> if (mergeRequest(*sipMsg) )
> {
> InfoLog (<< "Merged request: " << *sipMsg);
> return;
> }
> }
> *snip*
>
> If the From header-field-value is malformed, this will throw
> (unless something has already checked it).
>
> ======================================================================
> ==========
>
> in DialogUsageManager::mergeRequest() at line 1438 called by
> DialogUsageManager::incomingProcess() at line 1236
> *snip*
> assert(request.isRequest());
> assert(request.isExternal());
>
> if (!request.header(h_To).exists(p_tag))
> {
> if (mMergedRequests.count(MergedRequestKey(request,
> getMasterProfile()->checkReqUriInMergeDetectionEnabled())))
> {
> SipMessage failure;
> makeResponse(failure, request, 482, "Merged Request");
> failure.header(h_AcceptLanguages) = getMasterProfile()-
> >getSupportedLanguages();
> sendResponse(failure);
> return true;
> }
> }
> *snip*
>
> If the To header-field-value is malformed, an exception is thrown.
>
> ======================================================================
> ==========
>
> in MergedRequestKey::MergedRequestKey() at line 17
> called by DialogUsageManager::mergeRequest() at line 1440
> DialogUsageManager::incomingProcess() at line 1236
> *snip*
> MergedRequestKey::MergedRequestKey(const SipMessage& req, bool
> checkRequestUri) :
> mRequestUri(Data::from(req.header(h_RequestLine).uri())),
> mCSeq(Data::from(req.header(h_CSeq))),
> mTag(req.header(h_From).exists(p_tag) ? req.header(h_From).param
> (p_tag) : Data::Empty),
> mCallId(req.header(h_CallID).value()),
> mCheckRequestUri(checkRequestUri)
> {
> }
> *snip*
>
> If the Call-Id header-field-value is malformed, this will throw.
> (additionally, if the From header-field-value is malformed, we
> throw, but there is code that runs before this that will throw
> first. CSeq is safe, since it is checked for well-formedness by the
> stack, but it may be appropriate to account for this anyway in case
> things change)
>
> ======================================================================
> ==========
>
>
> Hitting any of these code-paths with a request will cause the
> stack to leak a TransactionState. Our problems do not end there,
> however. I haven't had time to look through all the possible
> feature-chains that can be invoked, but if any of them throw, the
> exception ends up making it to an even higher level (whatever is
> calling DialogUsageManager::process()), and we leak. Lastly, I have
> not gotten around to inspecting either of
> DialogUsageManager::processRequest() or
> DialogUsageManager::processResponse(). If either of these end up
> throwing, we are going to leak.
>
> We need to fix this stuff. I would take a crack at fixing it, but
> then again I am just getting my feet wet with DUM. I could break
> something unintentionally.
>
> Best regards,
> Byron Campen
>
>
>
>
>
> _______________________________________________
> resiprocate-devel mailing list
> resiprocate-devel at list.resiprocate.org
> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://list.resiprocate.org/pipermail/resiprocate-devel/attachments/20070307/3d5b30a0/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dumfixes2.patch
Type: application/octet-stream
Size: 4701 bytes
Desc: not available
URL: <http://list.resiprocate.org/pipermail/resiprocate-devel/attachments/20070307/3d5b30a0/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://list.resiprocate.org/pipermail/resiprocate-devel/attachments/20070307/3d5b30a0/attachment-0001.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 2423 bytes
Desc: not available
URL: <http://list.resiprocate.org/pipermail/resiprocate-devel/attachments/20070307/3d5b30a0/attachment.bin>
More information about the resiprocate-devel
mailing list