DXGL r389 - Code Review

Jump to navigation Jump to search
Repository:DXGL
Revision:r388‎ | r389 | r390 >
Date:00:11, 27 June 2013
Author:admin
Status:new
Tags:
Comment:
Move IsBadReadPointer function to separate source file.
Validate lplpDD in DirectDrawCreate and DirectDrawCreateEx.
Validate DirectDraw GUID values.
Validate lpDDSurfaceDesc2->dwSize in CreateSurface.
Modified paths:
  • /ddraw/ddraw.cpp (modified) (history)
  • /ddraw/ddraw.h (modified) (history)
  • /ddraw/ddraw.vcxproj (modified) (history)
  • /ddraw/ddraw.vcxproj.filters (modified) (history)
  • /ddraw/glDirectDraw.cpp (modified) (history)
  • /ddraw/glDirectDraw.h (modified) (history)
  • /ddraw/util.cpp (added) (history)
  • /ddraw/util.h (added) (history)

Diff [purge]

Index: ddraw/ddraw.cpp
@@ -16,6 +16,7 @@
1717 // Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
1818
1919 #include "common.h"
 20+#include "util.h"
2021 #include "ddraw.h"
2122 #include "texture.h"
2223 #include "glutil.h"
@@ -39,40 +40,6 @@
4041 glRenderer *renderer = NULL;
4142 glDirectDraw7 *dxglinterface = NULL;
4243
43 -/**
44 - * Tests if a pointer is valid for reading from. Compile in Visual C++ with /EHa
45 - * enabled Structed Exception Handling in C++ code, to prevent crashes on invalid
46 - * pointers.
47 - * @param ptr
48 - * Pointer to test for validity.
49 - * @return
50 - * Returns false if the pointer is valid, or true if an error occurs.
51 - */
52 -bool IsBadReadPointer(void *ptr)
53 -{
54 - TRACE_ENTER(1,14,ptr);
55 - char a;
56 - try
57 - {
58 - a = *(char*)ptr;
59 - if(a == *(char*)ptr)
60 - {
61 - TRACE_EXIT(21,0);
62 - return false;
63 - }
64 - else
65 - {
66 - TRACE_EXIT(21,1);
67 - return true;
68 - }
69 - }
70 - catch(...)
71 - {
72 - TRACE_EXIT(21,1);
73 - return true;
74 - }
75 -}
76 -
7744 void InitGL(int width, int height, int bpp, bool fullscreen, HWND hWnd, glDirectDraw7 *glDD7)
7845 {
7946 TRACE_ENTER(6,11,width,11,height,11,bpp,21,fullscreen,13,hWnd,14,glDD7);
@@ -191,6 +158,7 @@
192159 HRESULT WINAPI DirectDrawCreate(GUID FAR *lpGUID, LPDIRECTDRAW FAR *lplpDD, IUnknown FAR *pUnkOuter)
193160 {
194161 TRACE_ENTER(3,24,lpGUID,14,lplpDD,14,pUnkOuter);
 162+ if(!lplpDD) TRACE_RET(HRESULT,23,DDERR_INVALIDPARAMS);
195163 HRESULT ret;
196164 if(gllock || IsCallerOpenGL(_ReturnAddress()))
197165 {
@@ -292,6 +260,7 @@
293261 HRESULT WINAPI DirectDrawCreateEx(GUID FAR *lpGUID, LPVOID *lplpDD, REFIID iid, IUnknown FAR *pUnkOuter)
294262 {
295263 TRACE_ENTER(4,24,lpGUID,14,lplpDD,24,&iid,14,pUnkOuter);
 264+ if(!lplpDD) TRACE_RET(HRESULT,23,DDERR_INVALIDPARAMS);
296265 if(dxglinterface)
297266 {
298267 TRACE_EXIT(23,DDERR_DIRECTDRAWALREADYCREATED);
Index: ddraw/ddraw.h
@@ -58,6 +58,7 @@
5959
6060 extern DXGLCFG dxglcfg;
6161 extern bool gllock;
 62+extern const GUID device_template;
6263 extern DWORD timer;
6364 extern int vsyncstatus;
6465 class glRenderer;
Index: ddraw/ddraw.vcxproj
@@ -298,6 +298,7 @@
299299 <ClInclude Include="shaders.h" />
300300 <ClInclude Include="texture.h" />
301301 <ClInclude Include="trace.h" />
 302+ <ClInclude Include="util.h" />
302303 </ItemGroup>
303304 <ItemGroup>
304305 <ClCompile Include="ddraw.cpp" />
@@ -361,6 +362,7 @@
362363 <ClCompile Include="shaders.cpp" />
363364 <ClCompile Include="texture.cpp" />
364365 <ClCompile Include="trace.cpp" />
 366+ <ClCompile Include="util.cpp" />
365367 </ItemGroup>
366368 <ItemGroup>
367369 <ResourceCompile Include="ddraw.rc" />
Index: ddraw/ddraw.vcxproj.filters
@@ -128,6 +128,9 @@
129129 <ClInclude Include="trace.h">
130130 <Filter>Header Files</Filter>
131131 </ClInclude>
 132+ <ClInclude Include="util.h">
 133+ <Filter>Header Files</Filter>
 134+ </ClInclude>
132135 </ItemGroup>
133136 <ItemGroup>
134137 <ClCompile Include="ddraw.cpp">
@@ -214,6 +217,9 @@
215218 <ClCompile Include="trace.cpp">
216219 <Filter>Source Files</Filter>
217220 </ClCompile>
 221+ <ClCompile Include="util.cpp">
 222+ <Filter>Source Files</Filter>
 223+ </ClCompile>
218224 </ItemGroup>
219225 <ItemGroup>
220226 <ResourceCompile Include="ddraw.rc">
Index: ddraw/glDirectDraw.cpp
@@ -16,6 +16,7 @@
1717 // Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
1818
1919 #include "common.h"
 20+#include "util.h"
2021 #include "shaders.h"
2122 #include "ddraw.h"
2223 #include "glDirect3D.h"
@@ -569,6 +570,24 @@
570571 TRACE_ENTER(3,14,this,24,lpGUID,14,pUnkOuter);
571572 dxglinterface = this;
572573 initialized = false;
 574+ if(((ULONG_PTR)lpGUID > 2) && IsBadReadPointer(lpGUID))
 575+ {
 576+ error = DDERR_INVALIDPARAMS ;
 577+ TRACE_EXIT(-1,0);
 578+ return;
 579+ }
 580+ GUID guid;
 581+ if((ULONG_PTR)lpGUID > 2)
 582+ {
 583+ guid = *lpGUID;
 584+ guid.Data1 &= 0xFFFFFF00;
 585+ if(guid != device_template)
 586+ {
 587+ error = DDERR_INVALIDDIRECTDRAWGUID;
 588+ TRACE_EXIT(-1,0);
 589+ return;
 590+ }
 591+ }
573592 if(pUnkOuter)
574593 {
575594 error = DDERR_INVALIDPARAMS ;
@@ -585,28 +604,31 @@
586605 {
587606 TRACE_ENTER(1,14,this);
588607 dxglinterface = NULL;
589 - if(glD3D7) glD3D7->Release();
590 - RestoreDisplayMode();
591 - if(clippers)
 608+ if(initialized)
592609 {
593 - for(int i = 0; i < clippercount; i++)
 610+ if(glD3D7) glD3D7->Release();
 611+ RestoreDisplayMode();
 612+ if(clippers)
594613 {
595 - if(clippers[i]) clippers[i]->Release();
596 - clippers[i] = NULL;
 614+ for(int i = 0; i < clippercount; i++)
 615+ {
 616+ if(clippers[i]) clippers[i]->Release();
 617+ clippers[i] = NULL;
 618+ }
 619+ free(clippers);
597620 }
598 - free(clippers);
599 - }
600 - if(surfaces)
601 - {
602 - for(int i = 0; i < surfacecount; i++)
 621+ if(surfaces)
603622 {
604 - if(surfaces[i]) delete surfaces[i];
605 - surfaces[i] = NULL;
 623+ for(int i = 0; i < surfacecount; i++)
 624+ {
 625+ if(surfaces[i]) delete surfaces[i];
 626+ surfaces[i] = NULL;
 627+ }
 628+ free(surfaces);
606629 }
607 - free(surfaces);
 630+ if(renderer) delete renderer;
 631+ renderer = NULL;
608632 }
609 - if(renderer) delete renderer;
610 - renderer = NULL;
611633 TRACE_EXIT(-1,0);
612634 }
613635
@@ -802,6 +824,7 @@
803825 TRACE_EXIT(23,DD_OK);
804826 return DD_OK;
805827 }
 828+
806829 HRESULT WINAPI glDirectDraw7::CreateSurface(LPDDSURFACEDESC2 lpDDSurfaceDesc2, LPDIRECTDRAWSURFACE7 FAR *lplpDDSurface, IUnknown FAR *pUnkOuter)
807830 {
808831 TRACE_ENTER(4,14,this,14,lpDDSurfaceDesc2,14,lplpDDSurface,14,pUnkOuter);
@@ -808,6 +831,17 @@
809832 if(!this) TRACE_RET(HRESULT,23,DDERR_INVALIDOBJECT);
810833 if(!lpDDSurfaceDesc2) TRACE_RET(HRESULT,23,DDERR_INVALIDPARAMS);
811834 if(pUnkOuter) TRACE_RET(HRESULT,23,DDERR_INVALIDPARAMS);
 835+ if(lpDDSurfaceDesc2->dwSize < sizeof(DDSURFACEDESC2)) TRACE_RET(HRESULT,23,DDERR_INVALIDPARAMS);
 836+ TRACE_RET(HRESULT,23,CreateSurface2(lpDDSurfaceDesc2,lplpDDSurface,pUnkOuter));
 837+}
 838+
 839+
 840+HRESULT glDirectDraw7::CreateSurface2(LPDDSURFACEDESC2 lpDDSurfaceDesc2, LPDIRECTDRAWSURFACE7 FAR *lplpDDSurface, IUnknown FAR *pUnkOuter)
 841+{
 842+ TRACE_ENTER(4,14,this,14,lpDDSurfaceDesc2,14,lplpDDSurface,14,pUnkOuter);
 843+ if(!this) TRACE_RET(HRESULT,23,DDERR_INVALIDOBJECT);
 844+ if(!lpDDSurfaceDesc2) TRACE_RET(HRESULT,23,DDERR_INVALIDPARAMS);
 845+ if(pUnkOuter) TRACE_RET(HRESULT,23,DDERR_INVALIDPARAMS);
812846 if(primary && (lpDDSurfaceDesc2->ddsCaps.dwCaps & DDSCAPS_PRIMARYSURFACE) && (renderer->hRC == primary->hRC) )
813847 {
814848 if(primarylost)
@@ -1658,8 +1692,10 @@
16591693 {
16601694 TRACE_ENTER(4,14,this,14,lpDDSurfaceDesc,14,lplpDDSurface,14,pUnkOuter);
16611695 if(!this) TRACE_RET(HRESULT,23,DDERR_INVALIDOBJECT);
 1696+ if(!lpDDSurfaceDesc) TRACE_RET(HRESULT,23,DDERR_INVALIDPARAMS);
 1697+ if(lpDDSurfaceDesc->dwSize < sizeof(DDSURFACEDESC)) TRACE_RET(HRESULT,23,DDERR_INVALIDPARAMS);
16621698 LPDIRECTDRAWSURFACE7 lpDDS7;
1663 - HRESULT err = glDD7->CreateSurface((LPDDSURFACEDESC2)lpDDSurfaceDesc,&lpDDS7,pUnkOuter);
 1699+ HRESULT err = glDD7->CreateSurface2((LPDDSURFACEDESC2)lpDDSurfaceDesc,&lpDDS7,pUnkOuter);
16641700 if(err == DD_OK)
16651701 {
16661702 lpDDS7->QueryInterface(IID_IDirectDrawSurface,(LPVOID*) lplpDDSurface);
@@ -1844,8 +1880,10 @@
18451881 TRACE_ENTER(4,14,this,14,lpDDSurfaceDesc,14,lplpDDSurface,14,pUnkOuter);
18461882 if(!this) TRACE_RET(HRESULT,23,DDERR_INVALIDOBJECT);
18471883 if(!lplpDDSurface) TRACE_RET(HRESULT,23,DDERR_INVALIDPARAMS);
 1884+ if(!lpDDSurfaceDesc) TRACE_RET(HRESULT,23,DDERR_INVALIDPARAMS);
 1885+ if(lpDDSurfaceDesc->dwSize < sizeof(DDSURFACEDESC)) TRACE_RET(HRESULT,23,DDERR_INVALIDPARAMS);
18481886 LPDIRECTDRAWSURFACE7 lpDDS7;
1849 - HRESULT err = glDD7->CreateSurface((LPDDSURFACEDESC2)lpDDSurfaceDesc,&lpDDS7,pUnkOuter);
 1887+ HRESULT err = glDD7->CreateSurface2((LPDDSURFACEDESC2)lpDDSurfaceDesc,&lpDDS7,pUnkOuter);
18501888 if(err == DD_OK)
18511889 {
18521890 lpDDS7->QueryInterface(IID_IDirectDrawSurface,(LPVOID*) lplpDDSurface);
@@ -2053,8 +2091,10 @@
20542092 TRACE_ENTER(4,14,this,14,lpDDSurfaceDesc,14,lplpDDSurface,14,pUnkOuter);
20552093 if(!this) TRACE_RET(HRESULT,23,DDERR_INVALIDOBJECT);
20562094 if(!lplpDDSurface) TRACE_RET(HRESULT,23,DDERR_INVALIDPARAMS);
 2095+ if(!lpDDSurfaceDesc) TRACE_RET(HRESULT,23,DDERR_INVALIDPARAMS);
 2096+ if(lpDDSurfaceDesc->dwSize < sizeof(DDSURFACEDESC2)) TRACE_RET(HRESULT,23,DDERR_INVALIDPARAMS);
20572097 LPDIRECTDRAWSURFACE7 lpDDS7;
2058 - HRESULT err = glDD7->CreateSurface((LPDDSURFACEDESC2)lpDDSurfaceDesc,&lpDDS7,pUnkOuter);
 2098+ HRESULT err = glDD7->CreateSurface2((LPDDSURFACEDESC2)lpDDSurfaceDesc,&lpDDS7,pUnkOuter);
20592099 if(err == DD_OK)
20602100 {
20612101 lpDDS7->QueryInterface(IID_IDirectDrawSurface4,(LPVOID*) lplpDDSurface);
Index: ddraw/glDirectDraw.h
@@ -70,6 +70,7 @@
7171 HRESULT WINAPI EvaluateMode(DWORD dwFlags, DWORD *pSecondsUntilTimeout);
7272
7373 // internal functions
 74+ HRESULT CreateSurface2(LPDDSURFACEDESC2 lpDDSurfaceDesc2, LPDIRECTDRAWSURFACE7 FAR *lplpDDSurface, IUnknown FAR *pUnkOuter);
7475 HRESULT err() {return error;}
7576 void RemoveSurface(glDirectDrawSurface7 *surface);
7677 void GetSizes(LONG *sizes);
Index: ddraw/util.cpp
@@ -0,0 +1,58 @@
 2+// DXGL
 3+// Copyright (C) 2013 William Feely
 4+
 5+// This library is free software; you can redistribute it and/or
 6+// modify it under the terms of the GNU Lesser General Public
 7+// License as published by the Free Software Foundation; either
 8+// version 2.1 of the License, or (at your option) any later version.
 9+
 10+// This library is distributed in the hope that it will be useful,
 11+// but WITHOUT ANY WARRANTY; without even the implied warranty of
 12+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
 13+// Lesser General Public License for more details.
 14+
 15+// You should have received a copy of the GNU Lesser General Public
 16+// License along with this library; if not, write to the Free Software
 17+// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
 18+
 19+#include "common.h"
 20+#include "util.h"
 21+
 22+/**
 23+ * Tests if a pointer is valid for reading from. Compile in Visual C++ with /EHa
 24+ * enabled Structed Exception Handling in C++ code, to prevent crashes on invalid
 25+ * pointers.
 26+ * @param ptr
 27+ * Pointer to test for validity.
 28+ * @return
 29+ * Returns false if the pointer is valid, or true if an error occurs.
 30+ */
 31+bool IsBadReadPointer(void *ptr)
 32+{
 33+ TRACE_ENTER(1,14,ptr);
 34+ if(!ptr)
 35+ {
 36+ TRACE_EXIT(21,1);
 37+ return true;
 38+ }
 39+ char a;
 40+ try
 41+ {
 42+ a = *(char*)ptr;
 43+ if(a == *(char*)ptr)
 44+ {
 45+ TRACE_EXIT(21,0);
 46+ return false;
 47+ }
 48+ else
 49+ {
 50+ TRACE_EXIT(21,1);
 51+ return true;
 52+ }
 53+ }
 54+ catch(...)
 55+ {
 56+ TRACE_EXIT(21,1);
 57+ return true;
 58+ }
 59+}
Index: ddraw/util.h
@@ -0,0 +1,25 @@
 2+// DXGL
 3+// Copyright (C) 2013 William Feely
 4+
 5+// This library is free software; you can redistribute it and/or
 6+// modify it under the terms of the GNU Lesser General Public
 7+// License as published by the Free Software Foundation; either
 8+// version 2.1 of the License, or (at your option) any later version.
 9+
 10+// This library is distributed in the hope that it will be useful,
 11+// but WITHOUT ANY WARRANTY; without even the implied warranty of
 12+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
 13+// Lesser General Public License for more details.
 14+
 15+// You should have received a copy of the GNU Lesser General Public
 16+// License along with this library; if not, write to the Free Software
 17+// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
 18+
 19+#pragma once
 20+#ifndef _UTIL_H
 21+#define _UTIL_H
 22+
 23+
 24+bool IsBadReadPointer(void *ptr);
 25+
 26+#endif
\ No newline at end of file