Can you spot it?<\/p>\n
void p(const CString & str)
\n{
\nchar* endPtr = NULL;\u00a0
\nunsigned long x = strtoul(str.Mid(1, 2), 10, &endPtr);
\nif (*endPtr != ‘\\0’) {
\n\/\/ invaild input…
\n}
\n…
\n}<\/p>\n
\u00a0Using ATL’s CString, you can run this code and nothing will happen – the bug won’t be ‘triggered’. And more than that, everything will work as expected and correctly. But as most developers find bugs, running the software and fixing the crashes, this one didn’t come up to the surface… so you have a bug and you don’t know about it, how not fun. If you are really into C++ business it might be easy for you to spot this bug. But I’m not sure if it’s C++ to blame here, guess it’s not. If you take a look at the Mid method its declaration is as follows:<\/p>\n
CStringT Mid(int iFirst<\/span>, int nCount<\/span>) const;<\/p>\n \u00a0This means it returns a object<\/em> with the relevant slice of the original string. The destructor of this new object is immediately called, because its scope is a ‘parameter’. So now the endPtr points to freed memory. Yey. And then you access endPtr as you should…even if the memory is freed it is still in memory (because CString dynamically allocates space for its buffers), only AppVerifier forced it to be removed, thus\u00a0generating an access-violation and then it surfaced up and we fixed it. But, honestly, it was still tricky to spot this one.<\/p>\n \u00a0So now that you know the bug and why it happened, I can only say that you should beware of methods\/functions that return objects and has a very short scope, if at all. Not mentioning the copy constructor issues…\u00a0Of course, merely the scope isn’t to be\u00a0blamed, it’s the mixture of\u00a0short scope and a stale pointer (and dynamically allocated\u00a0memory),…To be honest, if the memory was local to the stack there was no way to detect this bug, unless the compiler would use the same stack space for different variables and they were written to between the call to strtoul and the access to the pointer. And even that isn’t reliable, but you could notice some weird mismatches between the input and that input validation…<\/p>\n \u00a0Any suggestions for tracking such bugs?<\/p>\n","protected":false},"excerpt":{"rendered":" Can you spot it? void p(const CString & str) { char* endPtr = NULL;\u00a0 unsigned long x = strtoul(str.Mid(1, 2), 10, &endPtr); if (*endPtr != ‘\\0’) { \/\/ invaild input… } … } \u00a0Using ATL’s CString, you can run this code and nothing will happen – the bug won’t be ‘triggered’. And more than that, […]<\/p>\n","protected":false},"author":1,"featured_media":0,"comment_status":"open","ping_status":"open","sticky":false,"template":"","format":"standard","meta":{"spay_email":"","jetpack_publicize_message":""},"categories":[20,19],"tags":[],"jetpack_featured_media_url":"","jetpack_publicize_connections":[],"jetpack_sharing_enabled":true,"jetpack_shortlink":"https:\/\/wp.me\/pbWKd-b","_links":{"self":[{"href":"https:\/\/www.ragestorm.net\/blogs\/index.php?rest_route=\/wp\/v2\/posts\/11"}],"collection":[{"href":"https:\/\/www.ragestorm.net\/blogs\/index.php?rest_route=\/wp\/v2\/posts"}],"about":[{"href":"https:\/\/www.ragestorm.net\/blogs\/index.php?rest_route=\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"https:\/\/www.ragestorm.net\/blogs\/index.php?rest_route=\/wp\/v2\/users\/1"}],"replies":[{"embeddable":true,"href":"https:\/\/www.ragestorm.net\/blogs\/index.php?rest_route=%2Fwp%2Fv2%2Fcomments&post=11"}],"version-history":[{"count":0,"href":"https:\/\/www.ragestorm.net\/blogs\/index.php?rest_route=\/wp\/v2\/posts\/11\/revisions"}],"wp:attachment":[{"href":"https:\/\/www.ragestorm.net\/blogs\/index.php?rest_route=%2Fwp%2Fv2%2Fmedia&parent=11"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/www.ragestorm.net\/blogs\/index.php?rest_route=%2Fwp%2Fv2%2Fcategories&post=11"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/www.ragestorm.net\/blogs\/index.php?rest_route=%2Fwp%2Fv2%2Ftags&post=11"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}