WingedSpectre wrote:Is this in case the server fails to follow the re-direct and keeps executing the script or is there some way a malicious user can do malicious things?
The header() -function sends data to the output buffer just like echo() does. Neither of them stops the execution, which seems to be what you want, right? It is easy to forget that your script is still executing and you may output something private in the end of the file without remembering this.
WingedSpectre wrote:I thought it wouldn't matter with ZIPs but I guess, either way, it's better to be safe than sorry

It does not matter what content you have there. It is an easy place to execute some XSS, for instance, although it only works for IE.
WingedSpectre wrote:In theory it will never happen because passwords and file locations will be updated by an administrator,
If an attacker has an access to your database, but not to your filesystem - he can modify the filename and therefore, this is one simple place to get an access to your filesystem, too. Besides, you never know if you alter your application and you allow other people upload files later. This might happen two years later, and at that point you no longer remember that your "protection" relied on the fact that no one else could alter your data.
WingedSpectre wrote:Did you mean it's not very random because MD5 is only 128-bit or because of the usage of uniqid() or rand()?
No, MD5 could be used, although not the best option. The function uniqid() essentially uses the same RNG (random number generator) than what rand() uses. They both generate their random output using an LCG (linear congruential generator), which generates poor random output. The same thing applies to mt_rand(). On Unix -like platforms, it is simple to fetch decent random data by reading the /dev/urandom file with fopen().
WingedSpectre wrote:Because in my code the Challenge part of the Challenge/Response is generated like this:
Code: Select all
$challenge = SHA256::hash(uniqid(mt_rand(), true));
I see that famous sentence everywhere. First you generate random data with the Mersenne Twister, which is a twisted GFSR (generalised feedback shift register) that produces poor random. Then you use uniqid() which generates a "unique" identifier based on a few facts such as the time, the time on microseconds, the current state of the internal LCG and the previously generated poor random data. After all this, you hash it with SHA-256. It is hard to say for sure, but I have estimated that about 10% of that data is crud. However, that line of code produces poor random results. The hash function is okay, but the entire contents inside of it should be replaced by some data gathered from the /dev/urandom (or CSP on Windows). Then your challenge generation approach meets all requirements sufficiently. I suggest you to use a strength of 256-bits as using more is useless due to the underlying 256-bit hash function. 256-bits is equal to 32 bytes. So use fopen() to read /dev/urandom and read 32 bytes and close the file. Now you have good random data to pass into your SHA-256.