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

Creating collections using webdav interface with '\' ignores everything after it instead of throwing an error. #16618

Closed
SergioBertolinSG opened this issue May 29, 2015 · 22 comments · Fixed by #16628 or #16755
Assignees
Milestone

Comments

@SergioBertolinSG
Copy link
Contributor

Steps to reproduce

  1. Using cadaver or other tool for interacting with webdav interface do:

mkcol hola\hallo

Expected behaviour

An error since '' is still a forbidden character.

Actual behaviour

cadaver says:
Creating `hola\hallo': succeeded.
And a folder called 'hola' is created.

Server configuration

Operating system:
Ubuntu 14.04

Web server:
Apache

Database:
MySQL

PHP version:
5.5.9

ownCloud version: (see ownCloud admin page)
{"installed":true,"maintenance":false,"version":"8.1.0.6","versionstring":"8.1 beta 2","edition":"Enterprise"}

Updated from an older ownCloud or fresh install:
Fresh

List of activated apps:


The content of config/config.php:


Are you using external storage, if yes which one: local/smb/sftp/...
No

Are you using encryption:
No

Logs


Client configuration

browser
Chrome 41

@davivel
Copy link

davivel commented May 29, 2015

Even further; seems WebDAV interface is not filtering any forbidden character at all. About 10 days ago we easily got an error messages playing with Poster and some MKCOLs with forbidden characters. I remember even a specific exception in the WebDAV response stating that the name included a forbidden character. What happened to it?

This is blocking two user stories for mobile clients right now.

cc @MTRichards @ggdiez

@SergioBertolinSG
Copy link
Contributor Author

I am just finding problems with backslash. What other characters failed before?

@davivel
Copy link

davivel commented May 29, 2015

Are you getting a rejection with some forbidden character? Which?

I can't remember what characters we used exactly, we made some test on-the-fly in the middle of a meeting.

@SergioBertolinSG
Copy link
Contributor Author

Since 8.1 these characters are a new feature and can be used too.

The forbidden characters were '', '/', '<', '>', ':', ',', '|' , '?' and '*'.

Now only '' and '*' are forbidden.

@davivel
Copy link

davivel commented May 29, 2015

But we are not receiving any error with '' through WebDAV. Though '' is a even more special case, I think, because in the web interface names are only reject if the full name is '' , not if '' is just a part of the name.

Besides, AFAIK, those characters are accepted if the storage where the server creates the folder allow them. But with external storages in file systems disallowing some of those characters, the server takes it into account an returns an error message. Am I wrong? Is this considered in the server tests, or did I misunderstand something?

So, maybe in the mentioned meeting we were playing with a server with external storage enabled to a FAT partition, or something similar.

@PVince81
Copy link
Contributor

Hmm, maybe the name check isn't kicking in ? @DeepDiver1975 @LukasReschke

Something to look into.

Sounds similar to #16401

@PVince81 PVince81 added this to the 8.1-current milestone May 29, 2015
@PVince81
Copy link
Contributor

Also possible on stable7, stable8. This is not a regression.

But I wonder if the code path for MKCOL is missing path validation ?

@PVince81
Copy link
Contributor

Can be reproduced with curl:

curl -X MKCOL 'http://root:admin@localhost/owncloud/remote.php/webdav/curl1%5ccurl2' 

There is some code that will automatically convert backslashes to slashes, and maybe somehow mkdir is called in a way that also creates parents.

I don't think this is a critical issue, if the backslash is the only allowed character.

@PVince81
Copy link
Contributor

I'll debug this

@PVince81 PVince81 self-assigned this May 29, 2015
@PVince81
Copy link
Contributor

Okay, basically this is what happens in stable7:

  1. Path arrives from Sabre with the backslash "curl1\curl2"
  2. The path is normalized first, which automatically converts backslashes to slashes (this was due to Windows paths in the past), becomes "curl1/curl2"
  3. Path "curl1/curl2" is validated for special chars: it doesn't have any
    So the code continues.

And on master it's the same.

For MKCOL Storage::verifyPath is not called.

@PVince81
Copy link
Contributor

Fix is here: #16628

It prevents "*" and also backslashes when creating directories.

@SergioBertolinSG
Copy link
Contributor Author

I am seeing a problem when trying to create a folder with just a backslash

curl -v -X MKCOL "http://user:password@host/remote.php/webdav/\\"
curl -v -X MKCOL 'http://user:password@host/remote.php/webdav/%5C'

It returns

<s:message>The resource you tried to create already exists</s:message>

I guess it should return invalid character.

With slash

curl -v -X MKCOL 'http://user:password@host/remote.php/webdav/aaa%2Fbbb'

this returns 404 not found, but aaa folder exists, i think bbb folder should be created isn't?
or at least since it is coded, perhaps it should return invalid character as well.

@purigarcia
Copy link

This is blocking two user stories for mobile clients right now.

@davivel
Copy link

davivel commented Jun 5, 2015

Mmm... about the backslash, I agree with @SergioBertolinSG

About the MKCOL with aaa%2Fbbb, I think it should finish in 400 Invalid path again, not in success; that command is trying to create a folder named "aaa/bbb" in the root folder. "/" is also a forbidden character for names of files or folders (of course). The web interface controls it correctly.

@PVince81
Copy link
Contributor

PVince81 commented Jun 5, 2015

I don't see how this minor issue would be blocking other tasks as this is only an error case, not a feature. Anyway, I'll have a look at this later today.

@PVince81
Copy link
Contributor

PVince81 commented Jun 5, 2015

@purigarcia is there a ticket for the blocked issues ?

@purigarcia
Copy link

Hi @PVince81 !
In our sprint, we have two issue (one for iOS and one for Android) to check that when you use forbidden characters, a message as "File contains at least one invalid character" pops up.
As webdav is not answering the correct respond when using / neither , we are not able to check that.

Do you mean ticket in the mobiles repositories? no, there are not. There is this issue.

@PVince81
Copy link
Contributor

PVince81 commented Jun 5, 2015

@SergioBertolinSG
Copy link
Contributor Author

@PVince81 Yes, that worked fine, thanks. I enabled that option in the apache virtualhost of owncloud.
So nothing to change in that case.

@PVince81
Copy link
Contributor

PVince81 commented Jun 5, 2015

Fix here for the MKCOL with backslash: #16755

In general the problem is that backslashes are removed (sanitized). In the MKCOL case, the internal node was actually "" or "/" instead or "/", so it tries to recreate the root directory which already exists, so it tells you that it already exists...

@davivel
Copy link

davivel commented Jun 5, 2015

@PVince81,

YOU ARE MY HERO.

Thanks

@purigarcia
Copy link

Thanks :), @PVince81

@lock lock bot locked as resolved and limited conversation to collaborators Aug 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants