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

Config: ASHX and Java connectors do not respect normalizeFilename option #131

Closed
dereks opened this issue Mar 21, 2017 · 6 comments
Closed

Comments

@dereks
Copy link
Contributor

dereks commented Mar 21, 2017

Only the PHP connector respects the normalizeFilename security option.

This option filters out control characters and whitespace from new file names. It should be enforced on the server.

Note that normalization has been done in the client-side Javascript in the past. But that is not secure -- this must be enforced on the server. (Never trust inputs from the client.) See #109 for more discussion about why this was removed from the client-side code.

@psolom
Copy link
Owner

psolom commented Mar 21, 2017

Once you implement features described in #106, #109 and #111 and I review them all, I will ask the maintainers of those connectors to make the updates. I would ask you to list all backward incompatible changes and other essentral recommendations at this topic when you finish with config and security stuff.

@dereks
Copy link
Contributor Author

dereks commented Mar 25, 2017

@servocoder

I would ask you to list all backward incompatible changes and other essentral recommendations at this topic when you finish with config and security stuff.

The changes in PR 139 are only config option changes, and they have been applied to all connectors (where I could find the given option name using grep). So, aside from the fact that some of the options were missing code already (like 'normalizeFilename'), I don't think PR 139 will require new, additional work from other connector maintainers.

For the new global blacklist/whitelist and allowNoExtension removal, I will make the change only to the PHP connector. Other connector maintainers will need to implement that feature independently.

The same is true with the pending security model changes. I will document my changes, but will only implement and test them on PHP. Assuming that you approve the model, other connector maintainers would need to implement changes for that too (as you have noted above).

Finally, I am still looking at removing culture from the server-side. The lang() method is used all over the place. I would like to remove this for all connectors, but that might not be practical. I think that is a change other connector maintainers will also need to implement independently.

@psolom
Copy link
Owner

psolom commented Mar 26, 2017

Thanks for keeping it up to date.

Finally, I am still looking at removing culture from the server-side.

I like your idea suggested in #109, see my answer: #109 (comment). We can ommit the fact that some connectors can have specific errors and force all connectors so use the global list of errors.

@psolom psolom added this to the 2.4.0 milestone Apr 1, 2017
@psolom
Copy link
Owner

psolom commented Apr 8, 2017

I am still looking at removing culture from the server-side. The lang() method is used all over the place.

I like your idea suggested in #109, see my answer: #109 (comment). We can ommit the fact that some connectors can have specific errors and force all connectors so use the global list of errors.

This feature has been released in v2.3.2

@psolom psolom removed this from the 2.4.0 milestone May 6, 2017
@psolom
Copy link
Owner

psolom commented May 7, 2017

@fabriceci (Java connector maintainer)
@richeflits (ASHX connector maintainer)

Please, take a look on the discussed subject.
Also I would ask you to update your connectors to the latest release (v2.4.0) and support all server-side and client-side features and configuration options. Feel free to ask if you have any questions or impediments. Thanks!

@psolom
Copy link
Owner

psolom commented Jul 15, 2017

Java connector should be already updated
@fabriceci, @gmkll take a look this issue to make sure, please.

@psolom psolom closed this as completed Jul 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants