Fix buffer overlow in case method docstring

[dipinhora]

Prior to this commit the add_docstring function had a
buffer overlow issue that was causing memory to be
clobbered.

This commit changes the add_docstring function to
use a printbuf to accumulate the string ensuring
no more buffer overflow issues.

Fixes #1609

[dipinhora]

@jemc Can you please test/confirm that this PR resolves #1609 for you?

In my testing, this resolves the error I get the same as the other two changes I mentioned in the ticket.

[jemc]

@dipinhora Yep, this solves the issue - sorry to assume that your hashmap implementation had to be the source of the problems - I guess the memory issue here was spilling over into the memory of the hashmap and causing the assert failures there.

Thanks so much for figuring it out!

[jemc]

Ready to merge when CI passes.

[chalcolith]

@dipinhora mea culpa; I wrote that function. Can you point out the buffer overflow to me for my re-education, please?

[dipinhora]

@jemc No problem. It took me many hours split across multiple days until I finally got lucky and figured out the real issue (which I only discovered because I switched to using malloc/free in pool.c along with using valgrind to try and ensure I wasn't doing something incorrect within the resize function of hash.c).

[dipinhora]

@kulibali No problem. It happens.

The following is what I understand to be occurring in the old code:

• print_params is always passed a buffer of 128 bytes with no checking to see if the concatenated string has overflowed the 128 bytes or not as more and more bytes are copied in resulting in overflow. This is because the last argument to strncat is supposed to be the # of bytes to copy and not the total size of the buffer. The code would have been correct if it was using strlcat instead of strncat. See: https://linux.die.net/man/3/strncat and http://stackoverflow.com/questions/6903997/how-can-i-use-strncat-without-buffer-overflow-concerns
• add_docstring calculates len using:

  const size_t len = id_len + 2 + m_ds_len + p_ds_len + c_ds_len + 2;
  

  Adding up to 8 extra bytes total (4 from the line quoted and another 2 as part of m_ds_len and the last 2 as part of p_ds_len, but all the string literals used potentially add up to 10 extra bytes (2 for "\n\n", 1 for "`", 1 for "(", 1 for ")", 2 for ": ", and 3 for "`: "). There's also a possibility where the 2 extra bytes from p_ds_len are not added to len if p_ds_len = 0 but the string literals those 2 extra bytes represent are added regardless of the value of p_ds_len.
  This means the buffer could be undersized in some circumstances. Combine this with the previous note about strncat and the buffer can overflow.

In the process of rewriting the logic for print_params to automagically resize the memory allocated, if needed, I noticed that printbuf is used in other passes and so decided to use that instead. Switching both functions to use printbuf simplifies the implementation and also resolves buffer overflow and potential string truncation on concatenation concerns.

[chalcolith]

Thanks! I should have noticed the use of printbuf elsewhere.

[jemc]

Thanks again!