Skip to content

Commit daef700

Browse files
AlexPeshkoffAlexPeshkoff
committed
Fixed #8429: Segfault when already destroyed callback interface is used
(cherry picked from commit 658abd2)
1 parent 00b648f commit daef700

File tree

15 files changed

+573
-27
lines changed

15 files changed

+573
-27
lines changed

‎doc/Using_OO_API.html‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3606,13 +3606,24 @@ <h1><font size="4" style="font-size: 14pt">Database encryption
36063606
is passed in both directions. The source of a key receives
36073607
dataLength bytes of data and may send up to bufferLength bytes into
36083608
buffer returning actual number of bytes placed into buffer.</font></p>
3609+
<li><p style="margin-bottom: 0cm"><font size="4" style="font-size: 14pt">
3610+
int getHashLength(StatusType* status) - returns length of hash of the keys provided by interface.
3611+
Hash is needed to let caller compare interfaces for equality. Empty (no keys)
3612+
interface should return hash length equal to zero.
3613+
</font></p>
3614+
<li><p style="margin-bottom: 0cm"><font size="4" style="font-size: 14pt">
3615+
void getHashData(StatusType* status, void* hash) - places hash of the keys provided by interface
3616+
into memory location defined by parameter <i>hash</i>. Should copy exact number of bytes
3617+
returned by getHashLength() call. Hash algorithm is chosen by plugin author
3618+
(in year 2024 SHA256 appears reasonable choice).
3619+
</font></p>
36093620
</ol>
36103621
<p style="margin-bottom: 0cm"><br/>
36113622

36123623
</p>
36133624
<p style="margin-bottom: 0cm"><a name="DbCryptInfo"></a><font size="4" style="font-size: 14pt">DbCryptInfo
36143625
interface is passed to DbCryptPlugin by engine. Plugin may save this
3615-
interface and use when needed to obtain additional informatio about
3626+
interface and use when needed to obtain additional information about
36163627
database.</font></p>
36173628
<ol>
36183629
<li><p style="margin-bottom: 0cm"><font size="4" style="font-size: 14pt">const

‎examples/dbcrypt/CryptApplication.cpp‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,14 +36,28 @@ class CryptKey : public ICryptKeyCallbackImpl<CryptKey, CheckStatusWrapper>
3636
{
3737
if (length > 0 && buffer)
3838
{
39-
char k = 0x5a;
4039
memcpy(buffer, &k, 1);
4140
fprintf(stderr, "\nTransfered key to server\n");
4241
}
4342
return 1;
4443
}
44+
45+
int getHashLength(Firebird::CheckStatusWrapper* status) override
46+
{
47+
return 1;
48+
}
49+
50+
void getHashData(Firebird::CheckStatusWrapper* status, void* h) override
51+
{
52+
memcpy(h, &k, 1);
53+
}
54+
55+
private:
56+
static const char k;
4557
};
4658

59+
const char CryptKey::k = 0x5a;
60+
4761
class App
4862
{
4963
public:

‎examples/dbcrypt/CryptKeyHolder.cpp‎

Lines changed: 50 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ class CryptKeyHolder : public IKeyHolderPluginImpl<CryptKeyHolder, CheckStatusWr
6969
public:
7070
explicit CryptKeyHolder(IPluginConfig* cnf) throw()
7171
: callbackInterface(this), named(NULL), tempStatus(master->getStatus()),
72-
config(cnf), key(0), owner(NULL)
72+
config(cnf), key(0), init(false), owner(NULL)
7373
{
7474
config->addRef();
7575
}
@@ -110,10 +110,13 @@ class CryptKeyHolder : public IKeyHolderPluginImpl<CryptKeyHolder, CheckStatusWr
110110
return owner;
111111
}
112112

113-
ISC_UCHAR getKey()
113+
const ISC_UCHAR& getKey()
114114
{
115-
if (!key)
115+
if (!init)
116+
{
116117
keyCallback(&tempStatus, NULL);
118+
init = true;
119+
}
117120

118121
return key;
119122
}
@@ -139,7 +142,7 @@ class CryptKeyHolder : public IKeyHolderPluginImpl<CryptKeyHolder, CheckStatusWr
139142

140143
unsigned int callback(unsigned int, const void*, unsigned int length, void* buffer)
141144
{
142-
ISC_UCHAR k = holder->getKey();
145+
const ISC_UCHAR& k = holder->getKey();
143146
if (!k)
144147
{
145148
return 0;
@@ -152,6 +155,36 @@ class CryptKeyHolder : public IKeyHolderPluginImpl<CryptKeyHolder, CheckStatusWr
152155
return 1;
153156
}
154157

158+
int getHashLength(Firebird::CheckStatusWrapper* status) override
159+
{
160+
const ISC_UCHAR& k = holder->getKey();
161+
if (!k)
162+
{
163+
ISC_STATUS err[] = {isc_arg_gds, isc_wish_list};
164+
status->setErrors2(2, err);
165+
166+
return -1;
167+
}
168+
169+
return 1;
170+
}
171+
172+
void getHashData(Firebird::CheckStatusWrapper* status, void* h) override
173+
{
174+
// here key value is returned by hash function as is
175+
// do not do it in production - use some hash function
176+
const ISC_UCHAR& k = holder->getKey();
177+
if (!k)
178+
{
179+
ISC_STATUS err[] = {isc_arg_gds, isc_wish_list};
180+
status->setErrors2(2, err);
181+
182+
return;
183+
}
184+
185+
memcpy(h, &k, 1);
186+
}
187+
155188
private:
156189
CryptKeyHolder* holder;
157190
};
@@ -172,6 +205,18 @@ class CryptKeyHolder : public IKeyHolderPluginImpl<CryptKeyHolder, CheckStatusWr
172205
return 1;
173206
}
174207

208+
int getHashLength(Firebird::CheckStatusWrapper* status) override
209+
{
210+
return 1;
211+
}
212+
213+
void getHashData(Firebird::CheckStatusWrapper* status, void* h) override
214+
{
215+
// here key value is returned by hash function as is
216+
// do not do it in production - use some hash function
217+
memcpy(h, &key, 1);
218+
}
219+
175220
~NamedCallback()
176221
{
177222
delete next;
@@ -188,6 +233,7 @@ class CryptKeyHolder : public IKeyHolderPluginImpl<CryptKeyHolder, CheckStatusWr
188233

189234
IPluginConfig* config;
190235
ISC_UCHAR key;
236+
bool init;
191237

192238
FbSampleAtomic refCounter;
193239
IReferenceCounted* owner;

‎src/include/firebird/FirebirdInterface.idl‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -931,6 +931,23 @@ interface CryptKeyCallback : Versioned
931931
// any further details.
932932
uint callback(uint dataLength, const void* data,
933933
uint bufferLength, void* buffer);
934+
935+
version: // 6.0, 5.0.2, 4.0.6
936+
937+
// NULL in attStatus means attach was successful.
938+
// DO_RETRY return will be ignored in this case, but plugin has a chance
939+
// to reflect internally success.
940+
[stub defaultAction]
941+
void dummy1(Status status);
942+
943+
// interface not needed any more
944+
[stub defaultAction]
945+
void dummy2();
946+
947+
// Produce something to be compared with other interface instance
948+
[notImplemented(-1)]
949+
int getHashLength(Status status);
950+
void getHashData(Status status, void* hash);
934951
}
935952

936953

‎src/include/firebird/IdlFbInterfaces.h‎

Lines changed: 121 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3763,6 +3763,10 @@ namespace Firebird
37633763
struct VTable : public IVersioned::VTable
37643764
{
37653765
unsigned (CLOOP_CARG *callback)(ICryptKeyCallback* self, unsigned dataLength, const void* data, unsigned bufferLength, void* buffer) throw();
3766+
void (CLOOP_CARG *dummy1)(ICryptKeyCallback* self, IStatus* status) throw();
3767+
void (CLOOP_CARG *dummy2)(ICryptKeyCallback* self) throw();
3768+
int (CLOOP_CARG *getHashLength)(ICryptKeyCallback* self, IStatus* status) throw();
3769+
void (CLOOP_CARG *getHashData)(ICryptKeyCallback* self, IStatus* status, void* hash) throw();
37663770
};
37673771

37683772
protected:
@@ -3776,13 +3780,62 @@ namespace Firebird
37763780
}
37773781

37783782
public:
3779-
static const unsigned VERSION = 2;
3783+
static const unsigned VERSION = 3;
37803784

37813785
unsigned callback(unsigned dataLength, const void* data, unsigned bufferLength, void* buffer)
37823786
{
37833787
unsigned ret = static_cast<VTable*>(this->cloopVTable)->callback(this, dataLength, data, bufferLength, buffer);
37843788
return ret;
37853789
}
3790+
3791+
template <typename StatusType> void dummy1(StatusType* status)
3792+
{
3793+
if (cloopVTable->version < 3)
3794+
{
3795+
StatusType::setVersionError(status, "ICryptKeyCallback", cloopVTable->version, 3);
3796+
StatusType::checkException(status);
3797+
return;
3798+
}
3799+
StatusType::clearException(status);
3800+
static_cast<VTable*>(this->cloopVTable)->dummy1(this, status);
3801+
StatusType::checkException(status);
3802+
}
3803+
3804+
void dummy2()
3805+
{
3806+
if (cloopVTable->version < 3)
3807+
{
3808+
return;
3809+
}
3810+
static_cast<VTable*>(this->cloopVTable)->dummy2(this);
3811+
}
3812+
3813+
template <typename StatusType> int getHashLength(StatusType* status)
3814+
{
3815+
if (cloopVTable->version < 3)
3816+
{
3817+
StatusType::setVersionError(status, "ICryptKeyCallback", cloopVTable->version, 3);
3818+
StatusType::checkException(status);
3819+
return -1;
3820+
}
3821+
StatusType::clearException(status);
3822+
int ret = static_cast<VTable*>(this->cloopVTable)->getHashLength(this, status);
3823+
StatusType::checkException(status);
3824+
return ret;
3825+
}
3826+
3827+
template <typename StatusType> void getHashData(StatusType* status, void* hash)
3828+
{
3829+
if (cloopVTable->version < 3)
3830+
{
3831+
StatusType::setVersionError(status, "ICryptKeyCallback", cloopVTable->version, 3);
3832+
StatusType::checkException(status);
3833+
return;
3834+
}
3835+
StatusType::clearException(status);
3836+
static_cast<VTable*>(this->cloopVTable)->getHashData(this, status, hash);
3837+
StatusType::checkException(status);
3838+
}
37863839
};
37873840

37883841
class IKeyHolderPlugin : public IPluginBase
@@ -13810,6 +13863,10 @@ namespace Firebird
1381013863
{
1381113864
this->version = Base::VERSION;
1381213865
this->callback = &Name::cloopcallbackDispatcher;
13866+
this->dummy1 = &Name::cloopdummy1Dispatcher;
13867+
this->dummy2 = &Name::cloopdummy2Dispatcher;
13868+
this->getHashLength = &Name::cloopgetHashLengthDispatcher;
13869+
this->getHashData = &Name::cloopgetHashDataDispatcher;
1381313870
}
1381413871
} vTable;
1381513872

@@ -13828,6 +13885,61 @@ namespace Firebird
1382813885
return static_cast<unsigned>(0);
1382913886
}
1383013887
}
13888+
13889+
static void CLOOP_CARG cloopdummy1Dispatcher(ICryptKeyCallback* self, IStatus* status) throw()
13890+
{
13891+
StatusType status2(status);
13892+
13893+
try
13894+
{
13895+
static_cast<Name*>(self)->Name::dummy1(&status2);
13896+
}
13897+
catch (...)
13898+
{
13899+
StatusType::catchException(&status2);
13900+
}
13901+
}
13902+
13903+
static void CLOOP_CARG cloopdummy2Dispatcher(ICryptKeyCallback* self) throw()
13904+
{
13905+
try
13906+
{
13907+
static_cast<Name*>(self)->Name::dummy2();
13908+
}
13909+
catch (...)
13910+
{
13911+
StatusType::catchException(0);
13912+
}
13913+
}
13914+
13915+
static int CLOOP_CARG cloopgetHashLengthDispatcher(ICryptKeyCallback* self, IStatus* status) throw()
13916+
{
13917+
StatusType status2(status);
13918+
13919+
try
13920+
{
13921+
return static_cast<Name*>(self)->Name::getHashLength(&status2);
13922+
}
13923+
catch (...)
13924+
{
13925+
StatusType::catchException(&status2);
13926+
return static_cast<int>(0);
13927+
}
13928+
}
13929+
13930+
static void CLOOP_CARG cloopgetHashDataDispatcher(ICryptKeyCallback* self, IStatus* status, void* hash) throw()
13931+
{
13932+
StatusType status2(status);
13933+
13934+
try
13935+
{
13936+
static_cast<Name*>(self)->Name::getHashData(&status2, hash);
13937+
}
13938+
catch (...)
13939+
{
13940+
StatusType::catchException(&status2);
13941+
}
13942+
}
1383113943
};
1383213944

1383313945
template <typename Name, typename StatusType, typename Base = IVersionedImpl<Name, StatusType, Inherit<ICryptKeyCallback> > >
@@ -13844,6 +13956,14 @@ namespace Firebird
1384413956
}
1384513957

1384613958
virtual unsigned callback(unsigned dataLength, const void* data, unsigned bufferLength, void* buffer) = 0;
13959+
virtual void dummy1(StatusType* status)
13960+
{
13961+
}
13962+
virtual void dummy2()
13963+
{
13964+
}
13965+
virtual int getHashLength(StatusType* status) = 0;
13966+
virtual void getHashData(StatusType* status, void* hash) = 0;
1384713967
};
1384813968

1384913969
template <typename Name, typename StatusType, typename Base>

0 commit comments

Comments
 (0)