Why does CLoginCookie::generateKey() seed the randomizer with srand() with the time every time (before the rand() is called) when a new login cookie is generated? That pretty much seems to defeat the rand() call...
Since the seed is random, the rand() still return a random number.
The main reason is that if you don't srand, you'll have the same random list and it's not what we want. And there's no init() function to srand the first time.
We could add a static bool inside the function to be srand onlt the first time the function i called but it's not really a problem to keep it as it.
The seed is the time, which isn't very random (and you're limited to one "random" per time unit). If you know a rough time when someone logged in (given a game where you can see how long someone's been playing this session), and you make a cookie right after to get the 'n' value, you'll "only" need to brute force a few thousand combinations to get someone's cookie, since the 'r' is always the first in the random list of 't'.
Sure, it's safe enough for now, but the risk is that the rand() can be too 'easily' reconstructed from some basic data, that shouldn't result in a security risk in normal cases.
Adding a static bool in the function to check and only seed once would be ok, I guess. But in that case also, since you have 'n' available you'll still know how many times rand() has been called, and if you also know the time that the first logincookie was made, you'll only be a few thousand guesses away again from a target's cookie. So the static 'n' should need to be randomized when it's initialized, in addition to only calling srand once, and then with that it should normally be a whole bit more safer. It would also be a good idea then to seed the randomizer from a high-res system timer instead of a one tick per second pure date time value. ^^
IF you have the userid,
IF you know EXACTLY, to the less than a second, when the userid will connect,
IF you know EXACTLY, to the second, what time is it on the server,
IF you have the same IP than the userid, then, perhaps,
IF your connection is faster than userid one, then perhaps, you can log in.
It's lot of if. I don't know how you can know the server time and you cannot know which time will be use for this specific user. Then, even if you have the right cookie, you have to connect with the same IP too and you also have to do that BEFORE the real player connects (that is less than a few seconds) and I still don't know how you can know that the user is connecting right now.
The CLoginCookie is clearly secure enough to achieve what it is used to.
You can do lot of things with IF, but it doesn't mean that it's insecure.
Not talking about the main connection to the game itself here. There can be other parts that use the cookie as well, remember.
- If I were to target someone, I'd probably be able to get hold of his uid (log files and whatever).
- Given a game where you can see how long someone's been online, or just by looking when your target pops in, and a few hundred brute force cookie guesses, that's passed easily.
- Login and look at own cookie to get a good part of the time. And you can assume that the server time is roughly correct. Or a case where someone also runs a forum or whatever on the same server, where you can see the time.
- I'm pretty sure that not all connections of some game would verify the address. Otherwise, get your target to login at a public internet location with shared ip.
- Other services linked to the game, such as certain ingame webpages.
It's not much work to make it a bit safer anyways. Was just interested if there was any real reason, other than "its good enough", why it's done this way.
Also allowing a user to seed the randomizer to a predictable value (given some service that generates login cookies, but also uses rand for other stuff), is pretty much a verrrry bad idea.
Something roughly in this style should be a few bits safer in more usage contexts anyways...
Why does CLoginCookie::generateKey() seed the randomizer with srand() with the time every time (before the rand() is called) when a new login cookie is generated? That pretty much seems to defeat the rand() call...
Since the seed is random, the rand() still return a random number.
The main reason is that if you don't srand, you'll have the same random list and it's not what we want. And there's no init() function to srand the first time.
We could add a static bool inside the function to be srand onlt the first time the function i called but it's not really a problem to keep it as it.
The seed is the time, which isn't very random (and you're limited to one "random" per time unit). If you know a rough time when someone logged in (given a game where you can see how long someone's been playing this session), and you make a cookie right after to get the 'n' value, you'll "only" need to brute force a few thousand combinations to get someone's cookie, since the 'r' is always the first in the random list of 't'.
Sure, it's safe enough for now, but the risk is that the rand() can be too 'easily' reconstructed from some basic data, that shouldn't result in a security risk in normal cases.
Adding a static bool in the function to check and only seed once would be ok, I guess. But in that case also, since you have 'n' available you'll still know how many times rand() has been called, and if you also know the time that the first logincookie was made, you'll only be a few thousand guesses away again from a target's cookie. So the static 'n' should need to be randomized when it's initialized, in addition to only calling srand once, and then with that it should normally be a whole bit more safer. It would also be a good idea then to seed the randomizer from a high-res system timer instead of a one tick per second pure date time value. ^^
IF you have the userid,
IF you know EXACTLY, to the less than a second, when the userid will connect,
IF you know EXACTLY, to the second, what time is it on the server,
IF you have the same IP than the userid, then, perhaps,
IF your connection is faster than userid one, then perhaps, you can log in.
It's lot of if. I don't know how you can know the server time and you cannot know which time will be use for this specific user. Then, even if you have the right cookie, you have to connect with the same IP too and you also have to do that BEFORE the real player connects (that is less than a few seconds) and I still don't know how you can know that the user is connecting right now.
The CLoginCookie is clearly secure enough to achieve what it is used to.
You can do lot of things with IF, but it doesn't mean that it's insecure.
Not talking about the main connection to the game itself here. There can be other parts that use the cookie as well, remember.
- If I were to target someone, I'd probably be able to get hold of his uid (log files and whatever).
- Given a game where you can see how long someone's been online, or just by looking when your target pops in, and a few hundred brute force cookie guesses, that's passed easily.
- Login and look at own cookie to get a good part of the time. And you can assume that the server time is roughly correct. Or a case where someone also runs a forum or whatever on the same server, where you can see the time.
- I'm pretty sure that not all connections of some game would verify the address. Otherwise, get your target to login at a public internet location with shared ip.
- Other services linked to the game, such as certain ingame webpages.
It's not much work to make it a bit safer anyways. Was just interested if there was any real reason, other than "its good enough", why it's done this way.
Also allowing a user to seed the randomizer to a predictable value (given some service that generates login cookies, but also uses rand for other stuff), is pretty much a verrrry bad idea.
Something roughly in this style should be a few bits safer in more usage contexts anyways...
// datetime in seconds
uint32 t = (uint32)time(NULL);
// counter
static uint32 n = 0;
// seed once
static bool seed = true;
if (seed)
{
// seed the randomizer
srand((uint)(CTime::getPerformanceTime() & 0xFFFFFFFF));
// start the counter at a random value
n = (uint32)rand();
seed = false;
}
// random value
uint32 r = (uint32)rand();
// increase counter
++n;
// 12bits for the time (in second) => loop in 1 hour
// 10bits for random => 1024 values
// 10bits for the inc number
return ((t & 0xFFF) << 20) | ((r & 0x3FF) << 10) | (r & 0x3FF);