Cloud broken in 6.7.7

[notepad-plus-plus]

Cloud broken in 6.7.7

[ariccio]

(I'm pretty sure that) The problem is that the buffer is NOT correctly null-terminated.

[rakekniven]

Can confirm this too. Just upgraded from 6.7.5 to 6.7.7 and being unable to select cloud option.

[ariccio]

My attempt at explaining this bug:
https://www.youtube.com/watch?v=D-mmjUhVDP8

It's actually fairly obvious if you run static analysis.

[ariccio]

What makes it really bad, is that there's a:

    } catch (...) {
        //printStr(TEXT("JsonCpp exception captured"));
    }

block, which via SEH catches the access violation, and thus "can cause process corruption and lead to bugs that are hard to find and fix" makes Notepad++ more reliable.

Those kinds of "catch-all exceptions" blocks are in 12 places in Notepad++, eleven of which swallow the exception and continue, which silently corrupts the user's data makes life easier for users.

[donho]

@Arbitel & @rakekniven
I'm working on fixing this issue.

[ariccio]

Changing:

int base64ToAscii(char* dest, const char* base64Str)

to:

int base64ToAscii(_Post_z_ _Post_readable_size_( return ) char* dest, _In_z_ const char* base64Str)

...and applying my static analysis patch, yields:

C6054   Zero termination missing
    String 'dest' might not be zero-terminated
    812 This expression may not zero-terminate 'dest'
    841 Skip this loop, (assume 'i<nbLoop' is false)
    880 Skip this branch, (assume 'padd==b64_1padded' is false)
    883 Skip this branch, (assume 'padd==b64_2padded' is false)
    812 'dest' should be zero-terminated but may not be

For reference: C6054

Sure enough, if I set a breakpoint at line 887, and Start Debugging:

ignore the message in the output window that's bitching about allocating executable memory.

...and the string is not null-terminated, in exactly those conditions!

[ariccio]

See: https://github.com/donho/notepad-plus-plus/pull/37

[donho]

@ariccio If you were you, I would try to reproduce the bug, instead shooting (or shouting :) ) in the dark.
The bug has been fixed and the fix will be in the next release.

[chadleong]

@donho Hmm, apparently it is still broken in 6.7.8.1

[ariccio]

Proof that denying an issue doesn't make it go away.