risk of data truncation and/or overrun when logging from a buffer bigger than GEARMAN_MAX_ERROR_SIZE

[yuzhiyuxia]

There is a risk of data truncation. ` gearmand_log(....) { .... *char log_buffer[GEARMAN_MAX_ERROR_SIZE2] = { 0 };* ...... if (Gearmand() and Gearmand()->log_fn) { Gearmand()->log_fn(log_buffer, verbose, (void )Gearmand()->log_context); } }

The callback function: static void _log(const char mesg, gearmand_verbose_t verbose, void context) { gearmand_log_info_st log_info= static_cast<gearmand_log_info_st >(context);

log_info->write(verbose, mesg); }

void write(gearmand_verbose_t verbose, const char *mesg) { if (opt_file) { char buffer[GEARMAN_MAX_ERROR_SIZE]; int buffer_length= snprintf(buffer, GEARMAN_MAX_ERROR_SIZE, "%7s %s\n", gearmand_verbose_name(verbose), mesg); ..... } } `

[p-alik]

GEARMAN_MAX_ERROR_SIZE 2048

log_buffer[GEARMAN_MAX_ERROR_SIZE*2] is defined in libgearman-server/log.cc gearmand purposes, buffer[GEARMAN_MAX_ERROR_SIZE]; in libgearman/log.cc for client and worker logging.

If I overseen something, please explain where is the risk?

[yuzhiyuxia]

@p-alik When you write a log, and set the log_file flag, the callback stack is gearmand_log_debug -> gearmand_log -> _log -> write. The callback stack is right? The size of log buffer is *GEARMAN_MAX_ERROR_SIZE2 in the gearmand_log function , but the size of log buffer is GEARMAN_MAX_ERROR_SIZE in the write function**

I begin to study the code. If you have any questions, please enlighten me.

[SpamapS]

These discussions are usually easier to have with a test that demonstrates the undesirable behavior.

[yuzhiyuxia]

@p-alik @SpamapS There is a test code: ` char pTestLog = (char)malloc(2051); for(int i = 0; i < 2047; i++){ pTestLog[i] = '0'; }

pTestLog[2047] = 'a'; pTestLog[2048] = 'b'; pTestLog[2049] = 'c'; pTestLog[2050] = 0; gearmand_log_debug(GEARMAN_DEFAULT_LOG_PARAM, "%s", pTestLog); free(pTestLog); ` There is the result:

From this picture, we can see there are some data is truncated!

[SpamapS]

Alright, this test proves there is definitely a bug!

However, let's be clear what it is. You've created a buffer bigger than GEARMAN_MAX_ERROR_SIZE, and passed it in, and we're reading more than GEARMAN_MAX_ERROR_SIZE from said buffer. Any reads from the source buffer should definitely stop at 2047 bytes (and substituted a null in for the 2048th byte).

I'd very much love to see your test code turned into a test in the gearmand suite, and a fix added to prevent such a mistake.

Thanks so much for looking closely at gearmand's code!