Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIX] Critical Bug - Form save: File with extension not allowed #593

Open
maofree opened this issue Jan 12, 2024 · 7 comments
Open

[FIX] Critical Bug - Form save: File with extension not allowed #593

maofree opened this issue Jan 12, 2024 · 7 comments

Comments

@maofree
Copy link

maofree commented Jan 12, 2024

Hi
Today I've seen that the form has a big problem after to send a message
I get this error

Form save: File with extension not allowed: contactform-06/01/2024 13:24:36.txt

1

why should the txt file be wrong?
It worked fine before, I think it's due to the latest plugin updates, which added this error

How is it possible to fix it?
all sites with grav have this problem. I use the latest versions

the email is sent correctly


I've seen that the problem is due from this condition inside Utils::checkFilename of Utils.php at line 989

|| strtr($filename, "\t\v\n\r\0\/", '_______') !== $filename

I think the problem is in the $filename variable because it is like so

contactform-06/01/2024 15:07:23.txt

but the real filename is so 2024 15:07:23.txt, because contactform-06 is a folder and 01 is another folder and the condition check the presence of slashes


it is not possible to use this line

$filename = $prefix . $this->udate($format, $raw_format) . $postfix . $ext;

with this condition

if (!Utils::checkFilename($filename)) { throw new RuntimeException(sprintf('Form save: File with extension not allowed: %s', $filename)); }

the filename includes folders with slashes in its name, so you should pass to that condition only the real filename or use a different condition


1

I suggest this solution

` $filename_array = $filename ? explode('/', $filename) : [];

            // Handle bad filenames.
            if (!Utils::checkFilename($filename_array[count($filename_array) -1])) {
                throw new RuntimeException(sprintf('Form save: File with extension not allowed: %s', $filename));
            }`

thanks

@maofree
Copy link
Author

maofree commented Jan 22, 2024

Is there anyone who handles this topic? This would solve a problem in sending emails, I think it's important

@rhukster
Copy link
Member

Sorry only me, and i've been sick.

It sounds like you are trying to have a custom path in the filename, and then you are trying to work-around the security checks that are there to prevent adding custom paths to filenames. The reason this is there now is due to a vulnerability that allowed arbitrary paths to be passed in via a user editable form.

So that said, it seems the error is correct, but the message is wrong, it's really the filename is not allowed, due to the path structure in it, not merely the extension.

In short the filename should not contain paths.

A better solution is to have a new path option that is added to the front of the filename, but is in some format that can be auto-generated, perhaps using PHP date tokens, So not completely arbitrary, something that can be validated.

In the meantime, i suggest just using the date in the filename rather than prefixing folders.

@maofree
Copy link
Author

maofree commented Jan 22, 2024

Hi
I'm sorry, yes I also got sick 23 weeks ago, now there are a lot of people with the flu

I don't think I did any customization, just something in the theme files for the form graphics. the problem arose due to an update made in the "form" plugin, in fact I saw that this check was added

1
2
3
4

do a check and you will see that in form/form.php the variable $filename also contains the paths to the file

with the version v7.2.1 this condition is not present and all is ok

as mentioned, the condition gives an error because the variable also contains the paths to the filename
I don't think I've made any changes to the file path, because I'm not interested in this type of change

thanks

@rhukster
Copy link
Member

What about your form definition in the form.md page file?

@maofree
Copy link
Author

maofree commented Jan 22, 2024

form.it.md

@rhukster
Copy link
Member

So in this part:

dateformat: 'd/m/Y H:i:s'

change it to:

dateformat: 'd-m-Y_H:i:s'

or someting similar

@maofree
Copy link
Author

maofree commented Jan 22, 2024

yes with this modification it doesn't give me errors

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants