Skip to content

Commit cec0859

Browse files
bclothierbclothier
committed
Fix AV by preventing access to non-constant VARDESC for external consumers. Introduce TypeInfoConstantsCollection to provide a collection of constants and remapping of the index to aid in enumeration by external consumers. Update several classes' documentations for clarity & cross-references.
1 parent 42e8747 commit cec0859

16 files changed

+466
-125
lines changed

‎Rubberduck.VBEEditor/ComManagement/TypeLibs/DebugInternal/TypeInfoWrapperTracer.cs

Lines changed: 237 additions & 58 deletions
Large diffs are not rendered by default.

‎Rubberduck.VBEEditor/ComManagement/TypeLibs/DebugInternal/TypeLibWrapperTracer.cs

Lines changed: 56 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,17 @@
1-
using System;
1+
#if DEBUG && TRACE_TYPEAPI
2+
using System;
23
using System.Runtime.CompilerServices;
34
using System.Runtime.InteropServices.ComTypes;
45
using Rubberduck.VBEditor.ComManagement.TypeLibs.Abstract;
56
using Rubberduck.VBEditor.ComManagement.TypeLibs.Unmanaged;
67

78
namespace Rubberduck.VBEditor.ComManagement.TypeLibs.DebugInternal
89
{
10+
/// <summary>
11+
/// Wraps the existing implementation so that we can trace the calls for
12+
/// diagnostics or debugging. See <see cref="TypeApiFactory"/> for
13+
/// creating a class to be traced. The class should not be created directly.
14+
/// </summary>
915
internal class TypeLibWrapperTracer : ITypeLibWrapper, ITypeLibInternal
1016
{
1117
private readonly ITypeLibWrapper _wrapper;
@@ -27,119 +33,129 @@ private static void After(string parameters = null, [CallerMemberName] string me
2733
System.Diagnostics.Debug.Print($"Leaving {nameof(ITypeLibWrapper)}::{methodName}; {parameters}");
2834
}
2935

30-
public int GetTypeInfoCount()
36+
private int GetTypeInfoCount()
3137
{
3238
Before();
3339
var result = _wrapper.GetTypeInfoCount();
3440
After($"{nameof(result)}: {result}");
3541
return result;
3642
}
3743

38-
public int GetTypeInfo(int index, IntPtr ppTI)
44+
int ITypeLibInternal.GetTypeInfoCount()
45+
{
46+
return GetTypeInfoCount();
47+
}
48+
49+
int ITypeLib.GetTypeInfoCount()
50+
{
51+
return GetTypeInfoCount();
52+
}
53+
54+
int ITypeLibInternal.GetTypeInfo(int index, IntPtr ppTI)
3955
{
4056
Before($"{nameof(index)}: {index}, {nameof(ppTI)}: {ppTI}");
4157
var result = _inner.GetTypeInfo(index, ppTI);
4258
After($"{nameof(result)}: {result}, {nameof(ppTI)}: {ppTI}");
4359
return result;
4460
}
4561

46-
public int GetTypeInfoType(int index, IntPtr pTKind)
62+
int ITypeLibInternal.GetTypeInfoType(int index, IntPtr pTKind)
4763
{
4864
Before($"{nameof(index)}: {index}, {nameof(pTKind)}: {pTKind}");
4965
var result = _inner.GetTypeInfoType(index, pTKind);
5066
After($"{nameof(result)}: {result}, {nameof(pTKind)}: {pTKind}");
5167
return result;
5268
}
5369

54-
public int GetTypeInfoOfGuid(ref Guid guid, IntPtr ppTInfo)
70+
int ITypeLibInternal.GetTypeInfoOfGuid(ref Guid guid, IntPtr ppTInfo)
5571
{
5672
Before($"{nameof(guid)}: {guid}, {nameof(ppTInfo)}: {ppTInfo}");
5773
var result = _inner.GetTypeInfoOfGuid(ref guid, ppTInfo);
5874
After($"{nameof(result)}: {result}, {nameof(ppTInfo)}: {ppTInfo}");
5975
return result;
6076
}
6177

62-
public int GetLibAttr(IntPtr ppTLibAttr)
78+
int ITypeLibInternal.GetLibAttr(IntPtr ppTLibAttr)
6379
{
6480
Before($"{nameof(ppTLibAttr)}: {ppTLibAttr}");
6581
var result = _inner.GetLibAttr(ppTLibAttr);
6682
After($"{nameof(result)}: {result}, {nameof(ppTLibAttr)}: {ppTLibAttr}");
6783
return result;
6884
}
6985

70-
public int GetTypeComp(IntPtr ppTComp)
86+
int ITypeLibInternal.GetTypeComp(IntPtr ppTComp)
7187
{
7288
Before($"{nameof(ppTComp)}: {ppTComp}");
7389
var result = _inner.GetTypeComp(ppTComp);
7490
After($"{nameof(result)}: {result}, {nameof(ppTComp)}: {ppTComp}");
7591
return result;
7692
}
7793

78-
public int GetDocumentation(int index, IntPtr strName, IntPtr strDocString, IntPtr dwHelpContext, IntPtr strHelpFile)
94+
int ITypeLibInternal.GetDocumentation(int index, IntPtr strName, IntPtr strDocString, IntPtr dwHelpContext, IntPtr strHelpFile)
7995
{
8096
Before($"{nameof(index)}: {index}, {nameof(strName)}: {strName}, {nameof(strDocString)}: {strDocString}, {nameof(dwHelpContext)}: {dwHelpContext}, {nameof(strHelpFile)}: {strHelpFile}");
8197
var result = _inner.GetDocumentation(index, strName, strDocString, dwHelpContext, strHelpFile);
8298
After($"{nameof(result)}: {result}, {nameof(strName)}: {strName}, {nameof(strDocString)}: {strDocString}, {nameof(dwHelpContext)}: {dwHelpContext}, {nameof(strHelpFile)}: {strHelpFile}");
8399
return result;
84100
}
85101

86-
public int IsName(string szNameBuf, int lHashVal, IntPtr pfName)
102+
int ITypeLibInternal.IsName(string szNameBuf, int lHashVal, IntPtr pfName)
87103
{
88104
Before($"{nameof(szNameBuf)}: {szNameBuf}, {nameof(lHashVal)}: {lHashVal}, {nameof(pfName)}: {pfName}");
89105
var result = _inner.IsName(szNameBuf, lHashVal, pfName);
90106
After($"{nameof(result)}: {result}, {nameof(pfName)}: {pfName}");
91107
return result;
92108
}
93109

94-
public int FindName(string szNameBuf, int lHashVal, IntPtr ppTInfo, IntPtr rgMemId, IntPtr pcFound)
110+
int ITypeLibInternal.FindName(string szNameBuf, int lHashVal, IntPtr ppTInfo, IntPtr rgMemId, IntPtr pcFound)
95111
{
96112
Before($"{nameof(szNameBuf)}: {szNameBuf}, {nameof(lHashVal)}: {lHashVal}, {nameof(ppTInfo)}: {ppTInfo}, {nameof(rgMemId)}: {rgMemId}, {nameof(pcFound)}: {pcFound}");
97113
var result = _inner.FindName(szNameBuf, lHashVal, ppTInfo, rgMemId, pcFound);
98114
After($"{nameof(result)}: {result}, {nameof(ppTInfo)}: {ppTInfo}, {nameof(rgMemId)}: {rgMemId}, {nameof(pcFound)}: {pcFound}");
99115
return result;
100116
}
101117

102-
public void GetTypeInfo(int index, out ITypeInfo ppTI)
118+
void ITypeLib.GetTypeInfo(int index, out ITypeInfo ppTI)
103119
{
104120
Before($"{nameof(index)}: {index}");
105121
_wrapper.GetTypeInfo(index, out var t);
106122
After($"{nameof(ppTI)}: {t?.GetHashCode()}");
107123
ppTI = t;
108124
}
109125

110-
public void GetTypeInfoType(int index, out TYPEKIND pTKind)
126+
void ITypeLib.GetTypeInfoType(int index, out TYPEKIND pTKind)
111127
{
112128
Before($"{nameof(index)}: {index}");
113129
_wrapper.GetTypeInfoType(index, out var t);
114130
After($"{nameof(pTKind)}: {t}");
115131
pTKind = t;
116132
}
117133

118-
public void GetTypeInfoOfGuid(ref Guid guid, out ITypeInfo ppTInfo)
134+
void ITypeLib.GetTypeInfoOfGuid(ref Guid guid, out ITypeInfo ppTInfo)
119135
{
120136
Before($"{nameof(guid)}: {guid}");
121137
_wrapper.GetTypeInfoOfGuid(ref guid, out var t);
122138
After($"{nameof(ppTInfo)}: {t?.GetHashCode()}");
123139
ppTInfo = t;
124140
}
125141

126-
public void GetLibAttr(out IntPtr ppTLibAttr)
142+
void ITypeLib.GetLibAttr(out IntPtr ppTLibAttr)
127143
{
128144
Before();
129145
_wrapper.GetLibAttr(out var t);
130146
After($"{nameof(ppTLibAttr)}: {t}");
131147
ppTLibAttr = t;
132148
}
133149

134-
public void GetTypeComp(out ITypeComp ppTComp)
150+
void ITypeLib.GetTypeComp(out ITypeComp ppTComp)
135151
{
136152
Before();
137153
_wrapper.GetTypeComp(out var t);
138154
After($"{nameof(ppTComp)}: {t?.GetHashCode()}");
139155
ppTComp = t;
140156
}
141157

142-
public void GetDocumentation(int index, out string strName, out string strDocString, out int dwHelpContext,
158+
void ITypeLib.GetDocumentation(int index, out string strName, out string strDocString, out int dwHelpContext,
143159
out string strHelpFile)
144160
{
145161
Before($"{nameof(index)}: {index}");
@@ -151,36 +167,46 @@ public void GetDocumentation(int index, out string strName, out string strDocStr
151167
strHelpFile = t4;
152168
}
153169

154-
public bool IsName(string szNameBuf, int lHashVal)
170+
bool ITypeLib.IsName(string szNameBuf, int lHashVal)
155171
{
156172
Before($"{nameof(szNameBuf)}: {szNameBuf}, {nameof(lHashVal)}: {lHashVal}");
157173
var result = _wrapper.IsName(szNameBuf, lHashVal);
158174
After($"{nameof(result)}: {result}");
159175
return result;
160176
}
161177

162-
public void FindName(string szNameBuf, int lHashVal, ITypeInfo[] ppTInfo, int[] rgMemId, ref short pcFound)
178+
void ITypeLib.FindName(string szNameBuf, int lHashVal, ITypeInfo[] ppTInfo, int[] rgMemId, ref short pcFound)
163179
{
164180
Before($"{nameof(szNameBuf)}: {szNameBuf}, {nameof(lHashVal)}: {lHashVal}, {ppTInfo}: {(ppTInfo == null ? "null" : "objects")}, {nameof(rgMemId)}: {(rgMemId == null ? "null" : "ints")}, {nameof(pcFound)}: {pcFound}");
165181
_wrapper.FindName(szNameBuf, lHashVal, ppTInfo, rgMemId, ref pcFound);
166182
After($"{ppTInfo}: {(ppTInfo == null ? "null" : "objects")}, {nameof(rgMemId)}: {(rgMemId == null ? "null" : "ints")}, {nameof(pcFound)}: {pcFound}");
167183
}
168184

169-
public void ReleaseTLibAttr(IntPtr pTLibAttr)
185+
private void ReleaseTLibAttr(IntPtr pTLibAttr)
170186
{
171187
Before($"{nameof(pTLibAttr)}: {pTLibAttr}");
172188
_wrapper.ReleaseTLibAttr(pTLibAttr);
173189
After();
174190
}
175191

192+
void ITypeLibInternal.ReleaseTLibAttr(IntPtr pTLibAttr)
193+
{
194+
ReleaseTLibAttr(pTLibAttr);
195+
}
196+
197+
void ITypeLib.ReleaseTLibAttr(IntPtr pTLibAttr)
198+
{
199+
ReleaseTLibAttr(pTLibAttr);
200+
}
201+
176202
public void Dispose()
177203
{
178204
Before();
179205
_wrapper.Dispose();
180206
After();
181207
}
182208

183-
public string Name
209+
string ITypeLibWrapper.Name
184210
{
185211
get
186212
{
@@ -191,7 +217,7 @@ public string Name
191217
}
192218
}
193219

194-
public string DocString
220+
string ITypeLibWrapper.DocString
195221
{
196222
get
197223
{
@@ -202,7 +228,7 @@ public string DocString
202228
}
203229
}
204230

205-
public int HelpContext
231+
int ITypeLibWrapper.HelpContext
206232
{
207233
get
208234
{
@@ -213,7 +239,7 @@ public int HelpContext
213239
}
214240
}
215241

216-
public string HelpFile
242+
string ITypeLibWrapper.HelpFile
217243
{
218244
get
219245
{
@@ -224,7 +250,7 @@ public string HelpFile
224250
}
225251
}
226252

227-
public bool HasVBEExtensions
253+
bool ITypeLibWrapper.HasVBEExtensions
228254
{
229255
get
230256
{
@@ -235,7 +261,7 @@ public bool HasVBEExtensions
235261
}
236262
}
237263

238-
public int TypesCount
264+
int ITypeLibWrapper.TypesCount
239265
{
240266
get
241267
{
@@ -246,7 +272,7 @@ public int TypesCount
246272
}
247273
}
248274

249-
public ITypeInfoWrapperCollection TypeInfos
275+
ITypeInfoWrapperCollection ITypeLibWrapper.TypeInfos
250276
{
251277
get
252278
{
@@ -257,7 +283,7 @@ public ITypeInfoWrapperCollection TypeInfos
257283
}
258284
}
259285

260-
public ITypeLibVBEExtensions VBEExtensions
286+
ITypeLibVBEExtensions ITypeLibWrapper.VBEExtensions
261287
{
262288
get
263289
{
@@ -268,7 +294,7 @@ public ITypeLibVBEExtensions VBEExtensions
268294
}
269295
}
270296

271-
public TYPELIBATTR Attributes
297+
TYPELIBATTR ITypeLibWrapper.Attributes
272298
{
273299
get
274300
{
@@ -279,7 +305,7 @@ public TYPELIBATTR Attributes
279305
}
280306
}
281307

282-
public int GetSafeTypeInfoByIndex(int index, out ITypeInfoWrapper outTI)
308+
int ITypeLibWrapper.GetSafeTypeInfoByIndex(int index, out ITypeInfoWrapper outTI)
283309
{
284310
Before($"{nameof(index)}: {index}");
285311
var result = _wrapper.GetSafeTypeInfoByIndex(index, out var t);
@@ -289,3 +315,4 @@ public int GetSafeTypeInfoByIndex(int index, out ITypeInfoWrapper outTI)
289315
}
290316
}
291317
}
318+
#endif

‎Rubberduck.VBEEditor/ComManagement/TypeLibs/TypeApiFactory.cs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,22 @@
1-
// #define TRACE_TYPEAPI
2-
3-
using System;
1+
using System;
42
using System.Runtime.InteropServices.ComTypes;
53
using Rubberduck.VBEditor.ComManagement.TypeLibs.Abstract;
64

5+
// TODO The tracers are broken - using them will cause a NRE inside the
6+
// unmanaged boundary. If we need to enable them for diagnostics, this needs
7+
// to be fixed first.
78
#if DEBUG && TRACE_TYPEAPI
89
using Rubberduck.VBEditor.ComManagement.TypeLibs.DebugInternal;
910
#endif
1011

1112
namespace Rubberduck.VBEditor.ComManagement.TypeLibs
1213
{
14+
/// <summary>
15+
/// Abstracts out the creation of the custom implementations of
16+
/// <see cref="ITypeLib"/> and <see cref="ITypeInfo"/>, mainly to
17+
/// make it easier to compose the implementation. For example, tracing
18+
/// can be enabled via the factory with appropriate compilation flags.
19+
/// </summary>
1320
internal static class TypeApiFactory
1421
{
1522
internal static ITypeLibWrapper GetTypeLibWrapper(IntPtr rawObjectPtr, bool addRef)

‎Rubberduck.VBEEditor/ComManagement/TypeLibs/TypeInfoConstantsCollection.cs

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Runtime.InteropServices.ComTypes;
4+
using Rubberduck.VBEditor.ComManagement.TypeLibs.Abstract;
5+
using Rubberduck.VBEditor.ComManagement.TypeLibs.Unmanaged;
6+
using TYPEATTR = System.Runtime.InteropServices.ComTypes.TYPEATTR;
7+
using VARDESC = System.Runtime.InteropServices.ComTypes.VARDESC;
8+
9+
namespace Rubberduck.VBEditor.ComManagement.TypeLibs
10+
{
11+
/// <summary>
12+
/// A special version of <see cref="TypeInfoVariablesCollection"/> which stores
13+
/// only the constants for the type library.
14+
/// </summary>
15+
/// <remarks>
16+
/// This aims to help work around the VBE type infos non-complaint
17+
/// behaviors of storing variables as a variable in a standard module
18+
/// which is considered a <see cref="TYPEKIND.TKIND_MODULE"/>. According
19+
/// to MS-OAUT open specifications "3.7.1.2 TYPEKIND Dependent Automation
20+
/// Type Description Elements", and "2.2.19 VARKIND Variable Kind Constants",
21+
/// the module should only contain a constant or a static variable which points
22+
/// to a coclass. In both cases, it is assumed that vardesc->lpvarValue->vt will
23+
/// always be valid. But that is not the case with a VBA type info representing a
24+
/// VBA standard module. Thus, this class is used to filter out all non constants
25+
/// from a module and thus conforms to the MS-OAUT specifications.
26+
///
27+
/// https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-oaut/7b1b8bd1-a067-4edb-9d72-6aa500d035a3
28+
/// https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-oaut/a0e9d463-51a2-49cc-8935-a65c9338d3df
29+
/// </remarks>
30+
internal class TypeInfoConstantsCollection : TypeInfoVariablesCollection
31+
{
32+
private Dictionary<int, int> _mapper;
33+
34+
public TypeInfoConstantsCollection(ITypeInfo parent, TYPEATTR attributes) :
35+
base(parent, attributes)
36+
{
37+
// External consumers won't actually know the real index of the underlying
38+
// VARDESC struct and will expect to be able to enumerate with a for loop.
39+
// We map the original index to a new position to make it easy to enumerate
40+
// the constants regardless of their positions in the actual index.
41+
_mapper = new Dictionary<int, int>();
42+
43+
for (var i = 0; i < attributes.cVars; i++)
44+
{
45+
parent.GetVarDesc(i, out var ppVarDesc);
46+
var varDesc = StructHelper.ReadStructureUnsafe<VARDESC>(ppVarDesc);
47+
48+
// VBA constants are "static".... go figure. We can still infer it is a
49+
// constant rather than a field by checking the lpvarValue
50+
if (varDesc.varkind == VARKIND.VAR_STATIC && varDesc.desc.lpvarValue != IntPtr.Zero)
51+
{
52+
_mapper.Add(_mapper.Count, i);
53+
}
54+
parent.ReleaseVarDesc(ppVarDesc);
55+
}
56+
}
57+
58+
public override int Count => _mapper.Count;
59+
60+
public override ITypeInfoVariable GetItemByIndex(int index) =>
61+
new TypeInfoVariable(Parent, _mapper[index]);
62+
63+
public int MappedIndex(int index) => _mapper[index];
64+
}
65+
}

0 commit comments

Comments
 (0)