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.
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.
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
<mailto: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 <mailto: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
<mailto: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
