AndrewPaglusch / FlashPaper

One-time encrypted password/secret sharing
MIT License
377 stars 60 forks source link

max_secret_length not quite working as expected #62

Closed pgrungi closed 2 years ago

pgrungi commented 2 years ago

First, I wanted to say thanks - this is a great little project. Being able to encrypt a message both via a simple web page and via a simple curl command is very handy.

I recently changed my max_secret_length in /settings.php to 10240. When I went to test it by pasting a giant blob of text into FlashPaper, I noticed a couple things:

  1. The input box limits me to the first 10240 characters of pasted text, as expected. That's good.
  2. Pressing the "encrypt" button afterward results in a generic "Input length too long" message. Maybe newline characters are being counted differently between the text input box validation and the strlen comparison in line 70 of /index.php? if ( strlen($_POST['secret']) > $settings['max_secret_length'] ) { throw new exception("Input length too long"); }
  3. Although you can customize error_secret_too_long in /settings.php, it doesn't seem to be referenced anywhere, e.g. in the exception message above.

In case it matters, the server environment is: FlashPaper release 2.0.0 PHP 7.4.28 Apache 2.4.6 Red Hat Enterprise Linux 7.9 on x86_64

The client environment for the test above was: Windows 10 on x86_64 Chrome 102.0.5005.115 and Firefox Nightly 103.0a1 ... so maybe some Windows line ending or UTF character encoding shenanigans are at play?

flashpaper-test-input-long.txt

AndrewPaglusch commented 2 years ago

Thank you for reaching out. I'm very happy to hear that you enjoy using FlashPaper!

You are correct about newline and other control characters being counted against the input length limit. I ran a small test here to show the character count of the file you provided, vs one that I generated with no control characters:

$ wc -c flashpaper-test-input-long.txt
  298998 flashpaper-test-input-long.txt

$ for i in {1..10240}; do printf "x" >> no_control_chars.txt; done
$ wc -c no_control_chars.txt
   10240 no_control_chars.txt

As you can see in this example, the file you attached in your post has 298,998 characters in it. The max_secret_length setting is a resource limitation so that visitors can not exhaust your server resources by making a gigantic secret POST. Knowing that, removing control characters (like newlines, spaces, tabs, etc) from the character count would defeat the intended purpose of that setting.

In regards to the error_secret_too_long variable not being used: Good catch! I opened #63 to get that corrected.

AndrewPaglusch commented 2 years ago

I just re-read your post again and realize that I missed the point you were making in your second item. Let me take a look into that and see why the client-side input validation and the strlen() function are behaving differently.

pgrungi commented 2 years ago

Awesome. Thank you!