single_php_filehost icon indicating copy to clipboard operation
single_php_filehost copied to clipboard

small security/privacy suggestion

Open Necrotyk opened this issue 8 months ago • 3 comments

Thank you for your work, I just have a small suggestion:

rand_str function using mt_rand isn't cryptographically secure, a motivated attacker could use a known filename to potentially enumerate other files in the folder, which could be bad if sensitive files have been uploaded in a self hosted server with the assumption nobody will guess the filenames and view them even if the folder can't be indexed or searched from a random user.

function rnd_str(int $len) : string
{
    // The character set for the random string
    $chars = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_';
    $max_idx = strlen($chars) - 1;
    $out = '';
 
    // Generate cryptographically secure random bytes
    $random_bytes = random_bytes($len);
 
    // Map each random byte to a character in the allowed set
    for ($i = 0; $i < $len; ++$i)
    {
        // ord() gets the integer value of the byte
        // The modulo operator maps it to an index within our character set
        $out .= $chars[ord($random_bytes[$i]) % ($max_idx + 1)];
    }
 
    return $out;
}

I know it's not a huge deal but it's better to be secure by default and let the user configure it however insecurely they want, in my opinion.

Consider using random_bytes() instead of mt_rand() and also increasing:

const MIN_ID_LENGTH = 3;

to at least 10 or more.

Necrotyk avatar Aug 13 '25 16:08 Necrotyk

the mt_rand thing is probably fair enough. imo you'd still have to guess at quite a lot of things and run into rate limits etc., but can't hurt to do things "correctly".

the MIN_ID_LENGTH is effectively a config, set it however you like. or use the id_length parameter at runtime.

Rouji avatar Aug 14 '25 01:08 Rouji

Thank you for your reply, and yeah I know it sounds like I'm nitpicking but I mean well. I'm just a paranoid, I suppose. And I laughed at the config statement, it's true. Felt a bit silly, but I was just coming at it from the scenario of some random person who clones repositories and runs everything default without thinking. So that's why it seems silly. :) Though I guess people who do such things should learn the hard way, though like I said this isn't really much of something to worry about, but I am having practice helping out other repositories. Thank you for being polite!

Necrotyk avatar Aug 14 '25 01:08 Necrotyk

the main point of this is more short URLs for throwaway uploads, than being a secure file store for your bank statements

anyway, PR for the random_bytes stuff would be nice

Rouji avatar Aug 14 '25 01:08 Rouji