Menu
▾
▴
SF.net SVN: alleg:[13967] allegro/branches/5.1
|
From: <tj...@us...> - 2010-11-30 22:09:02
|
Revision: 13967
http://alleg.svn.sourceforge.net/alleg/?rev=13967&view=rev
Author: tjaden
Date: 2010-11-30 22:08:55 +0000 (Tue, 30 Nov 2010)
Log Message:
-----------
Prevent 'DLL hijacking' security issue on Windows.
Pass absolute file names to LoadLibrary calls, to prevent it loading
DLLs from risky places such as the current directory or directories on
the PATH.
Modified Paths:
--------------
allegro/branches/5.1/addons/audio/dsound.cpp
allegro/branches/5.1/addons/native_dialog/win_dialog.c
allegro/branches/5.1/include/allegro5/platform/aintwin.h
allegro/branches/5.1/src/opengl/ogl_draw.c
allegro/branches/5.1/src/win/d3d_disp.cpp
allegro/branches/5.1/src/win/wjoydxnu.c
allegro/branches/5.1/src/win/wsystem.c
Modified: allegro/branches/5.1/addons/audio/dsound.cpp
===================================================================
--- allegro/branches/5.1/addons/audio/dsound.cpp 2010-11-30 13:52:21 UTC (rev 13966)
+++ allegro/branches/5.1/addons/audio/dsound.cpp 2010-11-30 22:08:55 UTC (rev 13967)
@@ -21,6 +21,7 @@
ALLEGRO_DEBUG_CHANNEL("audio-dsound")
#include "allegro5/internal/aintern_audio.h"
+#include "allegro5/internal/aintern_system.h"
/* This is used to stop MinGW from complaining about type-punning */
#define MAKE_UNION(ptr, t) \
@@ -34,10 +35,9 @@
/* DirectSound vars */
static const char* _al_dsound_module_name = "dsound.dll";
-static HMODULE _al_dsound_module = NULL;
+static void *_al_dsound_module = NULL;
static DIRECTSOUNDCREATE8PROC _al_dsound_create = (DIRECTSOUNDCREATE8PROC)NULL;
static IDirectSound8 *device;
-static HRESULT hr;
static char ds_err_str[100];
static int buffer_size_in_samples = 4*1024; // default
static int buffer_size; // in bytes
@@ -131,6 +131,7 @@
DWORD block1_bytes, block2_bytes;
const void *data;
const int bytes_per_sample = ex_data->bits_per_sample / 8;
+ HRESULT hr;
(void)self;
@@ -191,18 +192,20 @@
audio data to the device yet, however. */
static int _dsound_open()
{
+ HRESULT hr;
+
/* load DirectInput module */
- _al_dsound_module = LoadLibraryA(_al_dsound_module_name);
+ _al_dsound_module = _al_open_library(_al_dsound_module_name);
if (_al_dsound_module == NULL) {
ALLEGRO_ERROR("Failed to open '%s' library\n", _al_dsound_module_name);
return 1;
}
/* import DirectInput create proc */
- _al_dsound_create = (DIRECTSOUNDCREATE8PROC)GetProcAddress(_al_dsound_module, "DirectSoundCreate8");
+ _al_dsound_create = (DIRECTSOUNDCREATE8PROC)_al_import_symbol(_al_dsound_module, "DirectSoundCreate8");
if (_al_dsound_create == NULL) {
ALLEGRO_ERROR("DirectSoundCreate8 not in %s\n", _al_dsound_module_name);
- FreeLibrary(_al_dsound_module);
+ _al_close_library(_al_dsound_module);
return 1;
}
@@ -212,7 +215,7 @@
hr = _al_dsound_create(NULL, &device, NULL);
if (FAILED(hr)) {
ALLEGRO_ERROR("DirectSoundCreate8 failed\n");
- FreeLibrary(_al_dsound_module);
+ _al_close_library(_al_dsound_module);
return 1;
}
@@ -220,7 +223,7 @@
hr = device->SetCooperativeLevel(GetForegroundWindow(), DSSCL_PRIORITY);
if (FAILED(hr)) {
ALLEGRO_ERROR("SetCooperativeLevel failed\n");
- FreeLibrary(_al_dsound_module);
+ _al_close_library(_al_dsound_module);
return 1;
}
@@ -234,7 +237,7 @@
{
device->Release();
- FreeLibrary(_al_dsound_module);
+ _al_close_library(_al_dsound_module);
}
@@ -323,6 +326,7 @@
static int _dsound_load_voice(ALLEGRO_VOICE *voice, const void *data)
{
ALLEGRO_DS_DATA *ex_data = (ALLEGRO_DS_DATA *)voice->extra;
+ HRESULT hr;
LPVOID ptr1, ptr2;
DWORD block1_bytes, block2_bytes;
MAKE_UNION(&ex_data->ds8_buffer, LPDIRECTSOUNDBUFFER8 *);
@@ -379,6 +383,7 @@
static int _dsound_start_voice(ALLEGRO_VOICE *voice)
{
ALLEGRO_DS_DATA *ex_data = (ALLEGRO_DS_DATA *)voice->extra;
+ HRESULT hr;
MAKE_UNION(&ex_data->ds8_buffer, LPDIRECTSOUNDBUFFER8 *);
if (!voice->is_streaming) {
@@ -475,6 +480,7 @@
{
ALLEGRO_DS_DATA *ex_data = (ALLEGRO_DS_DATA *)voice->extra;
DWORD play_pos;
+ HRESULT hr;
hr = ex_data->ds8_buffer->GetCurrentPosition(&play_pos, NULL);
if (FAILED(hr))
@@ -489,6 +495,7 @@
static int _dsound_set_voice_position(ALLEGRO_VOICE *voice, unsigned int val)
{
ALLEGRO_DS_DATA *ex_data = (ALLEGRO_DS_DATA *)voice->extra;
+ HRESULT hr;
val *= ex_data->channels * (ex_data->bits_per_sample/8);
Modified: allegro/branches/5.1/addons/native_dialog/win_dialog.c
===================================================================
--- allegro/branches/5.1/addons/native_dialog/win_dialog.c 2010-11-30 13:52:21 UTC (rev 13966)
+++ allegro/branches/5.1/addons/native_dialog/win_dialog.c 2010-11-30 22:08:55 UTC (rev 13967)
@@ -27,7 +27,7 @@
static int wlog_class_registered = 0;
/* Handle of RichEdit module */
-static HMODULE wlog_rich_edit_module = 0;
+static void *wlog_rich_edit_module = 0;
/* Name of the edit control. Depend on system resources. */
static wchar_t* wlog_edit_control = L"EDIT";
@@ -351,17 +351,17 @@
/* Load RichEdit control. */
if (!wlog_rich_edit_module) {
- if ((wlog_rich_edit_module = LoadLibraryA("msftedit.dll"))) {
+ if ((wlog_rich_edit_module = _al_open_library("msftedit.dll"))) {
/* 4.1 and emulation of 3.0, 2.0, 1.0 */
wlog_edit_control = L"RICHEDIT50W"; /*MSFTEDIT_CLASS*/
wlog_unicode = true;
}
- else if ((wlog_rich_edit_module = LoadLibraryA("riched20.dll"))) {
+ else if ((wlog_rich_edit_module = _al_open_library("riched20.dll"))) {
/* 3.0, 2.0 */
wlog_edit_control = L"RichEdit20W"; /*RICHEDIT_CLASS*/
wlog_unicode = true;
}
- else if ((wlog_rich_edit_module = LoadLibraryA("riched32.dll"))) {
+ else if ((wlog_rich_edit_module = _al_open_library("riched32.dll"))) {
/* 1.0 */
wlog_edit_control = L"RichEdit"; /*RICHEDIT_CLASS*/
wlog_unicode = false;
@@ -379,7 +379,7 @@
(HINSTANCE)GetModuleHandle(NULL), textlog);
if (!hWnd) {
if (wlog_rich_edit_module) {
- FreeLibrary(wlog_rich_edit_module);
+ _al_close_library(wlog_rich_edit_module);
wlog_rich_edit_module = NULL;
}
UnregisterClassA("Allegro Text Log", (HINSTANCE)GetModuleHandle(NULL));
@@ -397,7 +397,7 @@
hWnd, NULL, (HINSTANCE)GetModuleHandle(NULL), NULL);
if (!hLog) {
if (wlog_rich_edit_module) {
- FreeLibrary(wlog_rich_edit_module);
+ _al_close_library(wlog_rich_edit_module);
wlog_rich_edit_module = NULL;
}
DestroyWindow(hWnd);
@@ -468,7 +468,7 @@
/* Release RichEdit module. */
if (wlog_rich_edit_module) {
- FreeLibrary(wlog_rich_edit_module);
+ _al_close_library(wlog_rich_edit_module);
wlog_rich_edit_module = NULL;
}
Modified: allegro/branches/5.1/include/allegro5/platform/aintwin.h
===================================================================
--- allegro/branches/5.1/include/allegro5/platform/aintwin.h 2010-11-30 13:52:21 UTC (rev 13966)
+++ allegro/branches/5.1/include/allegro5/platform/aintwin.h 2010-11-30 22:08:55 UTC (rev 13967)
@@ -112,6 +112,9 @@
extern bool _al_win_disable_screensaver;
+/* dynamic library loading */
+HMODULE _al_win_safe_load_library(const char *filename);
+
/* time */
void _al_win_init_time(void);
void _al_win_shutdown_time(void);
Modified: allegro/branches/5.1/src/opengl/ogl_draw.c
===================================================================
--- allegro/branches/5.1/src/opengl/ogl_draw.c 2010-11-30 13:52:21 UTC (rev 13966)
+++ allegro/branches/5.1/src/opengl/ogl_draw.c 2010-11-30 22:08:55 UTC (rev 13967)
@@ -45,10 +45,10 @@
return true;
}
else {
- glEnable(GL_BLEND);
- glBlendFunc(blend_modes[src_color], blend_modes[dst_color]);
- glBlendEquation(blend_equations[op]);
- return true;
+ glEnable(GL_BLEND);
+ glBlendFunc(blend_modes[src_color], blend_modes[dst_color]);
+ glBlendEquation(blend_equations[op]);
+ return true;
}
#else
if (d->ogl_extras->ogl_info.version >= _ALLEGRO_OPENGL_VERSION_1_4) {
Modified: allegro/branches/5.1/src/win/d3d_disp.cpp
===================================================================
--- allegro/branches/5.1/src/win/d3d_disp.cpp 2010-11-30 13:52:21 UTC (rev 13966)
+++ allegro/branches/5.1/src/win/d3d_disp.cpp 2010-11-30 22:08:55 UTC (rev 13967)
@@ -788,7 +788,7 @@
D3DDISPLAYMODE d3d_dm;
OSVERSIONINFO info;
- _al_d3d_module = LoadLibraryA(_al_d3d_module_name);
+ _al_d3d_module = _al_win_safe_load_library(_al_d3d_module_name);
if (_al_d3d_module == NULL) {
ALLEGRO_ERROR("Failed to open '%s' library\n", _al_d3d_module_name);
return false;
Modified: allegro/branches/5.1/src/win/wjoydxnu.c
===================================================================
--- allegro/branches/5.1/src/win/wjoydxnu.c 2010-11-30 13:52:21 UTC (rev 13966)
+++ allegro/branches/5.1/src/win/wjoydxnu.c 2010-11-30 22:08:55 UTC (rev 13967)
@@ -1068,7 +1068,7 @@
ASSERT(!STOP_EVENT);
/* load DirectInput module */
- _al_dinput_module = LoadLibraryA(_al_dinput_module_name);
+ _al_dinput_module = _al_win_safe_load_library(_al_dinput_module_name);
if (_al_dinput_module == NULL) {
ALLEGRO_ERROR("Failed to open '%s' library\n", _al_dinput_module_name);
joystick_dinput = NULL;
Modified: allegro/branches/5.1/src/win/wsystem.c
===================================================================
--- allegro/branches/5.1/src/win/wsystem.c 2010-11-30 13:52:21 UTC (rev 13966)
+++ allegro/branches/5.1/src/win/wsystem.c 2010-11-30 22:08:55 UTC (rev 13967)
@@ -34,9 +34,10 @@
#endif
#include <windows.h>
+#include <mmsystem.h>
+ALLEGRO_DEBUG_CHANNEL("system")
-#include <mmsystem.h>
/* FIXME: should we check for psapi _WIN32_IE and shlobj?
{ */
@@ -483,18 +484,147 @@
return true;
}
-static void *win_open_library(const char *filename)
+static HMODULE load_library_at_path(ALLEGRO_PATH *path, const char *filename)
{
- HINSTANCE lib = LoadLibrary(filename);
- if (!lib) {
+ const char *path_str;
+ HMODULE lib;
+
+ al_set_path_filename(path, filename);
+ path_str = al_path_cstr(path, '\\');
+
+ lib = LoadLibraryA(path_str);
+ if (lib) {
+ ALLEGRO_INFO("Loaded %s\n", path_str);
+ }
+ else {
DWORD error = GetLastError();
HRESULT hr = HRESULT_FROM_WIN32(error);
/* XXX do something with it */
(void)hr;
+ ALLEGRO_WARN("Failed to load %s (error: %ld)\n", path_str, error);
}
+
return lib;
}
+static bool is_build_config_name(const char *s)
+{
+ return s &&
+ ( 0 == strcmp(s, "Debug")
+ || 0 == strcmp(s, "Release")
+ || 0 == strcmp(s, "RelWithDebInfo")
+ || 0 == strcmp(s, "Profile"));
+}
+
+static bool same_dir(ALLEGRO_PATH *dir1, ALLEGRO_PATH *dir2)
+{
+ const char *s1;
+ const char *s2;
+ int i, n1, n2;
+
+ n1 = al_get_path_num_components(dir1);
+ n2 = al_get_path_num_components(dir2);
+ if (n1 != n2)
+ return false;
+
+ for (i = 0; i < n1; i++) {
+ s1 = al_get_path_component(dir1, i);
+ s2 = al_get_path_component(dir2, i);
+ if (strcmp(s1, s2) != 0)
+ return false;
+ }
+
+ s1 = al_get_path_drive(dir1);
+ s2 = al_get_path_drive(dir2);
+ if (!s1 || !s2 || strcmp(s1, s2) != 0)
+ return false;
+
+ return true;
+}
+
+static HMODULE maybe_load_library_at_cwd(ALLEGRO_PATH *path)
+{
+ char cwd_buf[MAX_PATH];
+ ALLEGRO_PATH *cwd;
+ const char *path_str;
+ HMODULE lib;
+
+ if (!is_build_config_name(al_get_path_tail(path)))
+ return NULL;
+
+ if (GetCurrentDirectoryA(sizeof(cwd_buf), cwd_buf) >= sizeof(cwd_buf)) {
+ ALLEGRO_WARN("GetCurrentDirectoryA failed\n");
+ return NULL;
+ }
+
+ al_drop_path_tail(path);
+ path_str = al_path_cstr(path, '\\');
+
+ cwd = al_create_path_for_directory(cwd_buf);
+ if (same_dir(path, cwd)) {
+ ALLEGRO_DEBUG("Assuming MSVC build directory, trying %s\n", path_str);
+ lib = LoadLibraryA(path_str);
+ }
+ al_destroy_path(cwd);
+
+ return lib;
+}
+
+/*
+ * Calling LoadLibrary with a relative file name is a security risk:
+ * see e.g. Microsoft Security Advisory (2269637)
+ * "Insecure Library Loading Could Allow Remote Code Execution"
+ */
+HMODULE _al_win_safe_load_library(const char *filename)
+{
+ char buf[MAX_PATH];
+ HMODULE lib;
+ bool msvc_only = false;
+
+ /* MSVC only: if the executable is in the build configuration directory,
+ * which is also just under the current directory, then also try to load the
+ * library from the current directory. This leads to less surprises when
+ * running example programs.
+ */
+#if defined(ALLEGRO_MSVC)
+ msvc_only = true;
+#endif
+
+ /* Try to load the library from the directory containing the running
+ * executable.
+ */
+ if (GetModuleFileName(NULL, buf, sizeof(buf)) < sizeof(buf)) {
+ ALLEGRO_PATH *path = al_create_path(buf);
+ lib = load_library_at_path(path, filename);
+ if (!lib && msvc_only) {
+ lib = maybe_load_library_at_cwd(path);
+ }
+ al_destroy_path(path);
+ if (lib)
+ return lib;
+ }
+
+ /* Try to load the library from the Windows system directory. */
+ if (GetSystemDirectoryA(buf, sizeof(buf)) < sizeof(buf)) {
+ ALLEGRO_PATH *path = al_create_path_for_directory(buf);
+ lib = load_library_at_path(path, filename);
+ al_destroy_path(path);
+ if (lib)
+ return lib;
+ }
+
+ /* Do NOT try to load the library from the current directory, or
+ * directories on the PATH.
+ */
+
+ return NULL;
+}
+
+static void *win_open_library(const char *filename)
+{
+ return _al_win_safe_load_library(filename);
+}
+
static void *win_import_symbol(void *library, const char *symbol)
{
ASSERT(library);
This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.
|