Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

An integer overflow bug in sdsrange caused by Incorrect types in send/recv can result in hangs with large pipelined send buffers #827

Closed
bwlewis opened this issue Jun 2, 2020 · 2 comments
Labels
Milestone

Comments

@bwlewis
Copy link

bwlewis commented Jun 2, 2020

Here is a crude example reproducing the errror. I will presently send a pull request fixing these issues.
Best regards.

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include "hiredis.h"

#define P 16777216     // Redis value length
#define argc 3         // Redis command + arguments
#define N 128          // Number of iterations
/* Compile with, for example, cc -I/usr/local/include/hiredis rediserr.c -lhiredis
 * Set N = 127, runs OK on all tested hiredis/Redis versions.
 * Set N = 128, OOM error or crash by hiredis 0.13.3, infinite hang on >= 0.14
 * The problem is a signed integer conversion logic bug in sdsrange, caused by using
 * int types instead of ssize_t in read/write code.
 */
int main()
{
  int i, j;
  const char x = 'x';
  char CMD[5] = {'R', 'P', 'U', 'S', 'H'};
  char *VAL = (char *)malloc(P * sizeof(char));
  if(!VAL) return -1;
  const size_t argvlen[] = {5, 1, P};
  const char *argv[argc] = {CMD, &x, VAL};
  memset((void *)VAL, 'x', P);
  redisReply *reply;
  redisContext *c = redisConnect("127.0.0.1", 6379);
  if (c == NULL || c->err) return -1;
  for(i = 0; i < N; ++i) {
    j = redisAppendCommandArgv(c, argc, argv, argvlen); 
    if(j != REDIS_OK || c->err) {
      printf("%s\n", c->errstr);
      exit(-1);
    }
  }
  /* retrieve command replies */
  for(i = 0; i < N; ++i) {
    j = redisGetReply(c, (void *)&reply);
    if(j != REDIS_OK || !reply) {
      printf("%s\n", c->errstr);
      exit(-1);
    }
    switch(reply->type) {
      case REDIS_REPLY_STATUS :
        printf("%s\n", reply->str);
        break;
      case REDIS_REPLY_ERROR :
        printf("%s\n", reply->str);
        break;
      case REDIS_REPLY_INTEGER :
        printf("%lld\n", reply->integer);
        break;
      default :
        printf("other reply\n");
    }
    freeReplyObject(reply);
  }
  free(VAL);
  return 0;
}
bwlewis pushed a commit to bwlewis/hiredis that referenced this issue Jun 2, 2020
@michael-grunder michael-grunder added this to the 1.0.0 milestone Jun 3, 2020
@michael-grunder
Copy link
Collaborator

See: #830

@bwlewis
Copy link
Author

bwlewis commented Jun 7, 2020

Looks good, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants