View Issue Details
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0003332||3 - Current Dev List||Bug||public||2019-06-07 09:49||2019-09-14 02:19|
|Target Version||Fixed in Version||126.96.36.199|
|Summary||0003332: QLM license code incorrectly casts string, ignores error return from Soraco function|
|Description||This code is in LicenseValidator::WriteProductProperties():|
string props = license->GetProductProperties(_bstr_t(""), _bstr_t(this->activationKey), &bstrResponse);
This code causes the application to crash because GetProductProperties() might return NULL, and NULL can't be successfully assigned to a std::string.
GetProductProperties() is declared to return a _bstr_t, which means an implicit cast is happening. That cast is dubious, since std::string is intended to represent a plain string (as compared to std::wstring which represents a wide dstring), while _bstr_t objects usually represent wide-character strings.
The correctness of the cast should be reviewed. The code should be fixed so that the error indicated by GetProductProperties() returning NULL is correctly handled and doesn't crash the application.
|Tags||No tags attached.|
|Module||SW License Mgmt|
|Sub-Module||SW License Client|
||Cleaned up the old code, not sure why the string was there any way, it was going BSTR->string->char*->then back to BSTR to be used. Looks like it works just fine as BSTR throughout, added the check for null, just returns false in that situation|
Changed to ....
_bstr_t bstrProps = license->GetProductProperties(_bstr_t(""), _bstr_t(this->activationKey), &bstrResponse);
otherwise it returns false.
Does that work for you Mike?
||Is this fixed and checked-in?|
Looks like this was fixed with this checkin on 2019-06-10:
When issues are fixed, we should be marking them "resolved" in Mantis. The checkin should have a comment that clearly indicates which Mantis issue the checkin addresses, and the Mantis issue should get a link to that checkin on the VSTS website.
The code in question was copied-and-pasted to another location. While this issue is fixed, we get to fix the same (or, at least, very similar) problem again in a different place. I've opened 3396 to track that issue.
On my laptop (with no QLM developer license), I crash at this calls stack starting Rig Control:
KernelBase.dll!_RaiseException@16() Unknown > HRDStation.dll!_CxxThrowException(void * pExceptionObject, const _s__ThrowInfo * pThrowInfo) Line 129 C++ HRDStation.dll!_com_raise_error(HRESULT hr, IErrorInfo * perrinfo) Line 18 C++ HRDStation.dll!_com_issue_errorex(HRESULT hr, IUnknown * punk, const _GUID & riid) Line 66 C++ HRDStation.dll!QlmLicenseLib::IQlmLicense::GetProductProperties(_bstr_t webServiceUrl, _bstr_t ActivationKey, wchar_t * * response) Line 3072 C++ HRDStation.dll!LicenseValidator::WriteProductProperties(ATL::CStringT<wchar_t,StrTraitMFC<wchar_t,ATL::ChTraitsCRT<wchar_t> > > &) Line 784 C++ HRDStation.dll!LicenseValidator::ValidateLicenseAtStartup(ATL::CStringT<wchar_t,StrTraitMFC<wchar_t,ATL::ChTraitsCRT<wchar_t> > > computerID, bool & needsActivation, ATL::CStringT<wchar_t,StrTraitMFC<wchar_t,ATL::ChTraitsCRT<wchar_t> > > & returnMsg) Line 220 C++ HRDStation.dll!HRDSoracoLicenser::CheckLicense(int bForceDlg) Line 114 C++ HRDStation.dll!_HRDInitStation() Line 320 C++ [Inline Frame] HRDStation.dll!CHRDStationApp::InitializeStation() Line 160 C++ HRDStation.dll!InitializeStation() Line 129 C++ HamRadioDeluxe.exe!CHamRadioDeluxeApp::InitInstance() Line 245 C++ HamRadioDeluxe.exe!AfxWinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, wchar_t * lpCmdLine, int nCmdShow) Line 37 C++ [Inline Frame] HamRadioDeluxe.exe!invoke_main() Line 118 C++ HamRadioDeluxe.exe!__scrt_common_main_seh() Line 288 C++ kernel32.dll!@BaseThreadInitThunk@12() Unknown ntdll.dll!__RtlUserThreadStart() Unknown ntdll.dll!__RtlUserThreadStart@8() Unknown
Sure, I should have a developer license. I asked for one and for whatever reason none is given.
But in the customer's hands, this same function might make a negative return and we would crash. We have no documentatoin about when it resturns fails or not, so it could happen to any customer at any time. Who knows?
I just ask that we write high-quality, resilient code, which doesn't crash with a negative return value.
||I dug into this a bit more. Looks like this is a problem in the TLB itself. When the underlying Interface returns NULL, it still returns S_OK. The wrapper code in the TLB tries to construct a _bstr_t from the NULL return value and pitches an exception. The TLB is provided by Soraco, so we'll have to get in touch with them about a fix. And, until then, hope that customers aren't getting NULL values back in the wild.|
I looked into it some last night too.
It definitely is like you said, tied to the dev license and running an api call in the debugger. I should have put 2+2 together earlier. I previously used the qlm admin API for porting the users over, and they had a very nice exception that said something like "Hey you can't run QLM admin api in the debugger"".
I threw process monitor on it, and tracked it down to the registry entry It is validating in the debugger.
We can catch the exception with...
catch (_com_error& err)
But even then we don't get much info other than something like unknown error.
As far as QLM is concerned I bet this is not a bug, but a feature.
I have an idea on how to handle this situation a bit better so we don't have to buy all of our devs a qlm license, I will try it out and report back.
For sure, the call returning an error is a feature -- they want to defend the software to not be run under a debugger.
But that feature has a bug because the NULL return value can't be represented in _bstr_t, as coded in their TLB. If they want to be able to return NULL to indicate a problem, the TLB shouldn't be using _bstr_t -- at least, not in the way that it does. I think the com_error that you're finding is from the _bstr_t constructor against the NULL return value, not from an unsuccessful HRESULT or natrual exception from the API.
If we're completely convinced that unlicenced developent is the only way to get a NULL back here, then I don't think it's a particularly high-priority issue for us. But I have no reference which explains when or why the return value might be NULL, or how to get that value out of the TLB call-side wrapper without trauma. As such, I'm still worried that some user-capable configuration ends up causing a crash.
So the isonline check was getting the valid QLM API error message on the non-licensed dev machines. So I made sure we didn't call the other web calls when isonline failed, which I should have been doing anyway.
IF we fail the isonline AND we are in DEBUG build AND we are in the debugger AND we are on a unlicensed dev machine, we will report the QLM error message one time.
Change set 5124
Note 1: If you do not have QLM licensed, you will have to license HRD outside the debugger one time. Subsequent runs will load fine.
Note 2: This should not be a risk to allow unlicensed HRD users to use the app by running in the debugger. The software will still require HRD to be licensed.
Thanks! Your fix enables development on machines without a QLM development license.
I'm still a bit worried about this API returning NULL for other reasons, but I don't think that's an issue with much priority.
|2019-06-07 09:49||K7ZCZ||New Issue|
|2019-06-07 09:49||K7ZCZ||Status||new => assigned|
|2019-06-07 09:49||K7ZCZ||Assigned To||=> DOUG|
|2019-06-09 19:02||WA9PIE||Module||All => QLM|
|2019-06-09 19:04||WA9PIE||Sub-Module||(select) => Software License Key System|
|2019-06-10 23:33||DOUG||Note Added: 0008036|
|2019-06-11 08:34||K7ZCZ||Relationship added||related to 0003345|
|2019-06-11 21:11||DOUG||Note Added: 0008052|
|2019-06-11 21:11||DOUG||Assigned To||DOUG => K7ZCZ|
|2019-06-13 14:47||WA9PIE||Module||QLM => SW License Mgmt|
|2019-06-15 11:21||WA9PIE||Description Updated||View Revisions|
|2019-06-15 11:21||WA9PIE||Sub-Module||Software License Key System => SW License Client|
|2019-06-22 11:15||WA9PIE||Note Added: 0008165|
|2019-07-21 06:38||K7ZCZ||Relationship added||related to 0003396|
|2019-07-21 06:46||K7ZCZ||Note Added: 0008254|
|2019-07-21 06:46||K7ZCZ||Status||assigned => resolved|
|2019-07-21 06:46||K7ZCZ||Resolution||open => fixed|
|2019-08-15 22:42||K7ZCZ||Note Added: 0008411|
|2019-08-15 22:48||K7ZCZ||Assigned To||K7ZCZ => DOUG|
|2019-08-16 12:15||K7ZCZ||Note Added: 0008413|
|2019-08-16 21:38||DOUG||Note Added: 0008414|
|2019-08-17 21:46||K7ZCZ||Note Added: 0008417|
|2019-08-18 20:36||DOUG||Note Added: 0008422|
|2019-08-20 12:28||K7ZCZ||Note Added: 0008424|
|2019-09-14 02:19||WA9PIE||Status||resolved => closed|
|2019-09-14 02:19||WA9PIE||Fixed in Version||=> 188.8.131.52|
|2019-09-14 02:19||WA9PIE||Summary||QLM license code incorrectly casts string, ignores error return from Socaro function => QLM license code incorrectly casts string, ignores error return from Soraco function|
|2019-09-14 02:19||WA9PIE||Testing||Not Started => Beta Successful|