Permalink
Browse files

Fixed CORE-5474: 'Restrict UDF' is not effective, because fbudf.so is…

… dynamically linked against libc
  • Loading branch information...
AlexPeshkoff committed Feb 6, 2017
1 parent 8621118 commit 8b2a9cb44bf6055e15f016d70a6842b8ada60375
Showing with 17 additions and 11 deletions.
  1. +0 −8 src/common/os/mod_loader.h
  2. +17 −3 src/common/os/posix/mod_loader.cpp
@@ -70,23 +70,15 @@ class ModuleLoader
/// Destructor
virtual ~Module() {}

#ifdef WIN_NT
const Firebird::PathName fileName;
#endif

protected:
/// The constructor is protected so normal code can't allocate instances
/// of the class, but the class itself is still able to be subclassed.
#ifdef WIN_NT
Module(MemoryPool& pool, const Firebird::PathName& aFileName)
: fileName(pool, aFileName)
{
}
#else
Module()
{
}
#endif

private:
/// Copy construction is not supported, hence the copy constructor is private
@@ -28,6 +28,7 @@
#include "firebird.h"
#include "../common/os/mod_loader.h"
#include "../common/os/os_utils.h"
#include "../common/os/path_utils.h"
#ifdef HAVE_UNISTD_H
#include <unistd.h>
#endif
@@ -40,8 +41,9 @@
class DlfcnModule : public ModuleLoader::Module
{
public:
DlfcnModule(void* m)
: module(m)
DlfcnModule(MemoryPool& pool, const Firebird::PathName& aFileName, void* m)
: ModuleLoader::Module(pool, aFileName),
module(m)
{}

~DlfcnModule();
@@ -109,7 +111,7 @@ ModuleLoader::Module* ModuleLoader::loadModule(const Firebird::PathName& modPath
system(command.c_str());
#endif

return FB_NEW_POOL(*getDefaultMemoryPool()) DlfcnModule(module);
return FB_NEW_POOL(*getDefaultMemoryPool()) DlfcnModule(*getDefaultMemoryPool(), modPath, module);
}

DlfcnModule::~DlfcnModule()
@@ -127,6 +129,18 @@ void* DlfcnModule::findSymbol(const Firebird::string& symName)

result = dlsym(module, newSym.c_str());
}

#ifdef HAVE_DLADDR
if (!PathUtils::isRelative(fileName))

This comment has been minimized.

@asfernandes

asfernandes Feb 6, 2017

Member

Why not normalize the path (as database names?) and test always? I see this now as very easy way to go wrong.

What if the .so is a symbolic link, will it work?

This comment has been minimized.

@hvlad

hvlad Feb 6, 2017

Member

Could (should) we use dlinfo( RTLD_DI_ORIGIN ) in the case of relative fileName ?

This comment has been minimized.

@AlexPeshkoff

AlexPeshkoff Feb 6, 2017

Member

As far as I remember UDF-related code (that seems to be the only place where user is free to enter arbitrary function name) UDF library name is always absolute. I.e. use of isRelative() condition is just a way to avoid something bad in other possible cases - like loading ICU modules.

Symbolic links in case of UDF library are always resolved.

dlinfo unfortunately is missing everywhere except linux (and possibly mac)

{
Dl_info info;
if (!dladdr(result, &info))
return NULL;
if (fileName != info.dli_fname)
return NULL;
}

This comment has been minimized.

@hvlad

hvlad Feb 6, 2017

Member

Is it possible that dlsym() returns entrypoint (of the same name) from another (previously) loaded module even when same entrypoint exists in given module ?

This comment has been minimized.

@asfernandes

asfernandes Feb 6, 2017

Member

More weird things can happen if one uses RTLD_GLOBAL to load libraries, but even without it, the thing of posix shared libraries is weird compared to everything else. I tried every LDD option without success to avoid looking at exe functions.

There is RTLD_NEXT that can be used to pass from library to library to get from the one you want.

I also tried the dlmopen (with namespace support) without good results.

This comment has been minimized.

@AlexPeshkoff

AlexPeshkoff Feb 6, 2017

Member

As far as I understand with parameters, used by us, not - only from module itself and needed to resolve it's external references.

#endif

return result;
}

0 comments on commit 8b2a9cb

Please sign in to comment.