Print

Please login or register.
1 Hour 1 Day 1 Week 1 Month Forever
Login with username, password and session length

[woutermense]

For instance folder support.

That is getting annoying. Especially because it's done for easy security. I think I found the offending lines of code in the source but I don't have a compiler to test it right now.

in src/webserver/src/WebServer.cpp (svn 9891)

Code: [Select]

334         // To prevent access to non-template images, we disallow use of paths in filenames.
335         wxString imgName = wxT("/") + wxFileName(Data.parsedURL.File()).GetFullName();

1853         // Strip out any path-component to prevent information leakage.
1854         wxString filename = wxFileName(Data.parsedURL.File()).GetFullName();

For example lines 1853 and 1854 could be replaced with

Code: [Select]

// Check if requested file is inside template base path
if (wxFileName(Data.parsedURL.File()).PrependDir(m_wwwroot).Normalize().GetPath().Contains(m_wwwroot))
{
wxString filename = wxFileName(Data.parsedURL.File());
}
else
{
// Strip out any path-component to prevent information leakage. (File probably won't exist?)
wxString filename = wxFileName(Data.parsedURL.File()).GetFullName();
}

So that it will check the path instead of truncate it

[wuischke]

Ay ay ay...well, that's something I stopped working on (I got distracted...multiple times) and I'm afraid nobody else did any more work.

But you did the right thing and wrote a patch. I have one small complaint: Please replace Contains with StartsWith. It's improbable that Contains will cause a real issue, but StartsWith makes this problem go away completely.

I'll just wait for somebody to test the code and see if it works fine and then I'll gladly commit your patch.

Code: [Select]

// Check if requested file is inside template base path
if (wxFileName(Data.parsedURL.File()).PrependDir(m_wwwroot).Normalize().GetPath().StartsWith(m_wwwroot)) {
wxString filename = wxFileName(Data.parsedURL.File());
} else {
// Strip out any path-component to prevent information leakage. (File probably won't exist?)
wxString filename = wxFileName(Data.parsedURL.File()).GetFullName();
}


[GonoszTopi]

I'm afraid nobody else did any more work.

You don't have to be. I did lots of work on it, but it's not in a state to be committable.

woutermense, your patch is nice and innocent but not innocuous (not to mention, incomplete ), thus I'm against committing it. You can safely use it do develop a template, but for official support you'll have to wait a bit more. I have two major TODOs, and one of them is webserver. So you can expect it soon, maybe this year if I'll have enough time.

[wuischke]

innocuous

Now you taught me a new word. Could you please tell us what may be harmful about the patch? Considered without context, I see nothing that could go wrong.

Edit: Maybe I should learn my old words properly first...

[woutermense]

Totally incomplete and untested. May not compile and I don't even know if I used the right variables.   But I got you talking about my idea and it is even already part of someone's todo. So I'm happy. Except now I have to find an English-Dutch dictionary

[GonoszTopi]

A nefarious template author could use symlinks in the template root to point to practically anywhere on the file system. The current implementation still leaves the hole for following symlinks that point to files outside the template root, but subdirectory support could effectively reveal the whole system to the attacker if the template contained a symlink to /, for example.

P.S.: Don't put your dictionaries away

[wuischke]

Good thinking. IIRC I wanted to put all template files inside a zip-archive and only allow access to file inside the zip. The main reason was ease of installation, but this would solve such problems, too. But memory usage and access speed might be negatively affected.

P.S. I have 75cm of dictionaries and grammars right behind me.

[woutermense]

And a measuring tape?

I hadn't thought about the symlinks and after looking around in the wx documents I can think of a lot of situations where I wouldn't know how the library works.  Anyway, I think everyone should run the daemon and the webserver within a special user account. Too bad security is also partly our responsibility . Does apache take any measures to keep you inside the wwwroot? Or is that purely done by file permissions?

[GonoszTopi]

Does apache take any measures to keep you inside the wwwroot?

IIRC it does.

[lfroen]

Does apache take any measures to keep you inside the wwwroot?

IIRC it does.

Unless you use PHP/Java/etc

[^marcell^]

I am not sure if this makes any sense for aMuleWeb, but have you guys considered chroot?

[GonoszTopi]

I am not sure if this makes any sense for aMuleWeb, but have you guys considered chroot?

Sure.

Only the root user can perform a chroot.