On 07/28/2011 03:40 AM, Varun Chandramohan wrote:
> On Thursday, July 28, 2011 12:51:47 am Nick Wagner wrote:
>> The SLPDPredicateTreeNode structure does seem to have a problem.  If we left
>> the larger-sized array, guards should still be added to ensure that neither
>> strncpy will step off the end of the member 'storage'.  Others may have
>> differing opinions, but I'd change storage to be a char* and alloc it to the
>> right size (tag_len + value_len + terminating nulls), as long as we make
>> sure that it's freed appropriately.  We're allocating all the nodes in the
>> tree anyway.
>>
>> --Nick
>>
> I think we can do that.  If all are ok with it, can we make this change?
It seems that the extra space for the attributes is already taken care 
of in allocating the memory for ppNode (slpd_predicate:1645). So Nick's 
proposed change shouldn't be necessary. But we still need to find out 
why it doesn't work properly yet...

Can we be certain that the 'storage' field is actually exactly at the 
end of the SLPPredicateTreeNode structure?

BR,
     Roel
>
> - Varun
>
>> On Wed, Jul 27, 2011 at 1:27 AM, Varun Chandramohan<
>> var...@linux.vnet.ibm.com>  wrote:
>>
>>> **
>>>
>>> Hi Folks,
>>>
>>>   The test team reported a bug. Iam pasting the bug analysis. They seem to
>>> have found the problem as well with a temporary fix. However i they want our
>>> opinion on the fix. Please advice.
>>>
>>> Using the following command:
>>>
>>> slptool findsrvs service:test$ID "(foo=value1)"
>>>
>>>   this will generat the overflow in createPredicateParseTree doing an 
>>> strncpy -
>>>
>>> line 1656 slpd_predicate.c
>>>
>>>   When reading the comments around "operator" it appears it is using the 
>>> operator
>>>
>>> 2 characters as a place to copy the attribute name.  The attribute name can 
>>> be
>>>
>>> very large.  This is the code:
>>>
>>>         /* Finished with "operator" now - just use as temporary pointer to 
>>> assist
>>>
>>> with copying the
>>>
>>>         * attribute name (lhs) and required value (rhs) into the node
>>>
>>>         */
>>>
>>>        operator = (*ppNode)->nodeBody.comparison.storage;
>>>
>>>        strncpy(operator, lhs, lhs_len);
>>>
>>>        operator[lhs_len] = '\0';
>>>
>>>   operator is now the pointer of "storage" in:
>>>
>>>   slpd_predicate.h
>>>
>>>   typedef struct __SLPDPredicateTreeNode
>>>
>>> {
>>>
>>>     SLPDPredicateTreeNodeType nodeType;
>>>
>>>     struct __SLPDPredicateTreeNode *next;     /* next node in a combination 
>>> */
>>>
>>>     union {
>>>
>>>        struct __SLPDPredicateLogicalBody
>>>
>>>        {
>>>
>>>           struct __SLPDPredicateTreeNode *first;
>>>
>>>        } logical;
>>>
>>>        struct __SLPDPredicateComparisonBody
>>>
>>>        {
>>>
>>>           size_t tag_len;
>>>
>>>           char *tag_str;
>>>
>>>           size_t value_len;
>>>
>>>           char *value_str;
>>>
>>>           char storage[2];
>>>
>>>        } comparison;
>>>
>>>     } nodeBody;
>>>
>>> } SLPDPredicateTreeNode;
>>>
>>>   Copying of attributes onto 2char array fails though doesn't fail in older
>>>
>>> builds so I am not sure if build options or strictness has changed.
>>>
>>>   Since there were no cleanup routines if the pointer was malloced, I just
>>>
>>> increased -->  storage[200] and the testcase runs without fail and the area 
>>> will
>>>
>>> get freed with the structure.
>>>
>>>   Is this the best way out?
>>>
>>>   Regards,
>>>
>>> Varun
>>>
>>>
>>>
>>> ------------------------------------------------------------------------------
>>> Got Input?   Slashdot Needs You.
>>> Take our quick survey online.  Come on, we don't ask for help often.
>>> Plus, you'll get a chance to win $100 to spend on ThinkGeek.
>>> http://p.sf.net/sfu/slashdot-survey
>>> _______________________________________________
>>> Openslp-devel mailing list
>>> Openslp-devel@lists.sourceforge.net
>>> https://lists.sourceforge.net/lists/listinfo/openslp-devel
>>>
>>>
> ------------------------------------------------------------------------------
> Got Input?   Slashdot Needs You.
> Take our quick survey online.  Come on, we don't ask for help often.
> Plus, you'll get a chance to win $100 to spend on ThinkGeek.
> http://p.sf.net/sfu/slashdot-survey
> _______________________________________________
> Openslp-devel mailing list
> Openslp-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openslp-devel


------------------------------------------------------------------------------
Got Input?   Slashdot Needs You.
Take our quick survey online.  Come on, we don't ask for help often.
Plus, you'll get a chance to win $100 to spend on ThinkGeek.
http://p.sf.net/sfu/slashdot-survey
_______________________________________________
Openslp-devel mailing list
Openslp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openslp-devel

Reply via email to