Fix buffer-overflow in sha256 string construction

[adam-rl]

Need to leave room for the last null-byte.

What this PR does / why we need it: Fixes a buffer overflow assertion in Release mode

Please check the following list:

• Does the affected code have corresponding tests, e.g. unit test, E2E test?
• Does this change require a documentation update?
• Does this introduce breaking changes that would require an announcement or bumping the major version?
• Do all new files have an appropriate license header?

[northtyphoon]

@adam-rl can you elebrate a little bit what the issue is? how did you run into the buffer overflow error?

[juliusl]

I don't think that constructor needs a null terminator? Since the length is passed explicitly?

[adam-rl]

sprintf will write a null terminating byte to the end of the string. This means that 3 bytes are written for each sprintf call: the 2 hex digits and a null-byte. The last byte of the sha256 will write past the end of the stack-allocated buffer unless accounted for with an extra byte.

If there was a variable declared after the buffer in the stack, it would be partially overwritten with a zero.

If you build on ubuntu in Release mode, -D_FORTIFY_SOURCE is enabled which causes the call to sprintf to abort.

[adam-rl]

@juliusl the issue isn't in copying the buffer to a std::string, that part is fine. The issue is in the final sprintf call of the loop.

[liulanzheng]

sprintf will write a null terminating byte to the end of the string. This means that 3 bytes are written for each sprintf call: the 2 hex digits and a null-byte. The last byte of the sha256 will write past the end of the stack-allocated buffer unless accounted for with an extra byte.

If there was a variable declared after the buffer in the stack, it would be partially overwritten with a zero.

If you build on ubuntu in Release mode, -D_FORTIFY_SOURCE is enabled which causes the call to sprintf to abort.

Thanks, sprintf indeed needs an extra character.