search

opentibia / server

An open source server for the MMORPG Tibia.
GNU General Public License v2.0
414 stars 149 forks source link

The setKey(filename) function is broken in several ways #22

Closed d33tah closed 10 years ago

d33tah commented 10 years ago

01:21:49 sarnold $ d33tah: wtf do lines 101, 102, 103 do??

01:24:31 sarnold $ d33tah: are you one of the maintainers of this codebase? 01:25:03 d33tah $ nope 01:25:06 sarnold $ damn 01:25:12 d33tah $ but i think i'll contact them 01:26:55 sarnold $ d33tah: please do, RSA::setKey() has multiple problems: (a) fails to close the file (b) delete on automatic arrays (htf does that even compile btw?) (c) no error checking on fgets() $ (d) improper use of fgets() (see the bit about "fgets() reads in at most one less than size characters..." in the manpage)

marksamman commented 10 years ago

I have fixed three of the problems mentioned by sarnold (a, b and c).

I'm unsure about the last one though. Does that mean the variables p, q and d should be 513 bytes (for fgets to put the terminating NULL byte at 512)?

d33tah commented 10 years ago

You can ask the fellow, he's online on #crypto, irc.freenode.net.

setharnold commented 10 years ago

Indeed, I had thought that 511 bytes would be a strange number of bytes to read for these parameters; 512 bytes of data seemed more likely, so setting the arrays to 513 bytes, so fgets() could read all (presumed) 512 bytes, and still have one array location left over for the trailing NUL byte. But if there really are just 511 bytes to read, this is fine as-is.

Thanks