Hi Paolo,
On Thu, May 17, 2012 at 9:37 PM, Paolo Tosco <paolo.to...@unito.it> wrote:
> Dear Craig,
>
> if you read more carefully my post you will see I have *perfectly* clear
> the difference between the size of a structure and the length of the
> string. Using strncpy instead of strcpy is the first rule to avoid a buffer
> overflow - strcpy is deprecated by all good programming practices, since if
> the source-string, though null-terminated, is longer than the size of the
> destination buffer, it will cause a buffer overflow, a segmentation fault
> and a possible exploit for malicious attacks. My fix was meant to rule out
> that possibility, since it limits the size of the copied buffer to the size
> of the destination buffer.
>
My apologies, you are absolutely right. The right way to do it is to have
a declared constant that defines the maximum string length and then ensure
that the strncpy() always uses that.
>
> However, I leave to the OpenBabel developers the decision to the best
> patch to be adopted. Anyway, the origin of the formal charge issue has been
> found, which is the most important aspect of the story.
>
Exactly.
Cheers,
Craig
>
> Best regards,
> Paolo
>
> On 05/18/2012 01:53 AM, Craig James wrote:
>
> Hi Paolo,
>
> I think you (and the original author) have mixed up the size of the
> structures with the length of the string. There's no need to do a sizeof()
> or to define a constant OB_ATOM_TYPE_LENGTH.
>
> Here's what I think is needed ... it's actually simpler than the
> original. Just use strcpy() instead of strncpy(), because these are all
> NULL-terminated strings.
>
> bool OBTypeTable::Translate(char *to, const char *from)
> {
> if (!_init)
> Init();
>
> bool rval;
> string sto,sfrom;
> sfrom = from;
> rval = Translate(sto,sfrom);
> strcpy(to, sto.c_str());
>
> return(rval);
> }
>
> My only worry is that the (char *)to parameter passed in has to be long
> enough to hold the (char*)from parameter. I assume it is.
>
> Craig
>
> On Thu, May 17, 2012 at 4:06 PM, Paolo Tosco <paolo.to...@unito.it> wrote:
>
>> Actually that sizeof in Translate (data.cpp) is plain wrong, since it
>> refers to sizeof(char *) which is 8 on a 64-bit OS and 4 on a 32-bit OS;
>> build the attached test.c with -m64 and -m32 and run to verify that:
>>
>> $ gcc -m64 test.c -o test64
>> $ ./test64
>> This is really 6: 6
>> This is machine-dependent: 8
>>
>> $ gcc -m32 test.c -o test32
>> $ ./test32
>> This is really 6: 6
>> This is machine-dependent: 4
>>
>>
>> Probably Chris has a 32-bit Windows machine, that's why it works.
>> So the rel fix is not increasing to 8 the size of _type in OBAtom
>> (atom.h), but rather defining OBATOM_TYPE_LEN = 6 in atom.h, setting
>> _type[OBATOM_TYPE_LEN] in OBAtom and replacing sizeof(to) with
>> OBATOM_TYPE_LEN in bool OBTypeTable::Translate(char *to, const char *from).
>> A new patch is attached.
>>
>> Cheers,
>> p.
>>
>> --
>> ==========================================================
>> Paolo Tosco, Ph.D. Phone: +39 011 6707680
>> Department of Drug Science Fax: +39 011 6707687
>> and Technology Mob: +39 348 5537206
>> Via Pietro Giuria, 9
>> 10125 Torino, Italy
>> http://open3dalign.org
>> E-mail: paolo.to...@unito.it http://open3dqsar.org
>> ==========================================================
>>
>>
>>
>>
>> ------------------------------------------------------------------------------
>> Live Security Virtual Conference
>> Exclusive live event will cover all the ways today's security and
>> threat landscape has changed and how IT managers can respond. Discussions
>> will include endpoint security, mobile security and the latest in malware
>> threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
>> _______________________________________________
>> OpenBabel-discuss mailing list
>> OpenBabel-discuss@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/openbabel-discuss
>>
>>
>
>
> --
> ==========================================================
> Paolo Tosco, Ph.D. Phone: +39 011 6707680
> Department of Drug Science Fax: +39 011 6707687
> and Technology Mob: +39 348 5537206
> Via Pietro Giuria, 9
> 10125 Torino, Italy
> http://open3dalign.org
> E-mail: paolo.to...@unito.it http://open3dqsar.org
> ==========================================================
>
>
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
OpenBabel-discuss mailing list
OpenBabel-discuss@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openbabel-discuss
