mjs icon indicating copy to clipboard operation
mjs copied to clipboard

buffer over-read in strtod

Open pellepl opened this issue 5 years ago • 0 comments

Hi, we've found an issue when running following script:

let i1 = JSON.parse("42\0");
print(i1);
let i2 = JSON.parse("42");
print(i2);

The second print sometimes prints extra numbers after 42. I've tracked this down to here. Here strtod is used on token->ptr. This pointer is however a non-null-terminated string, which is the problem. Sometimes the memory after the string contains valid data with regard to strtod which makes strtod parsing happily past intended buffer. ´strtod´ really should stop parsing after token->len bytes.

I've reproduced it in following example, where I also redefined malloc to mymalloc in mjs.c.

// file bug.c
#include <stdio.h>
#include <string.h>
#include "mjs.h"

void *mymalloc(size_t d) {
  void *p = malloc(d+8);
  memset(p, '8', d+8);
  return p;
}

int main(int argc, char **argv) {
  mjs_val_t res;
  struct mjs *mjs = mjs_create();
  const char *script = ""
  "let i1 = JSON.parse(\"42\\0\"); \n"
  "print(i1); // OK: Prints 42     \n"
  "let i2 = JSON.parse(\"42\");    \n"
  "print(i2); // BAD: Prints 42888 \n"
  ""; 
  mjs_err_t err =  mjs_exec(mjs, script, &res);
  printf("err:%d\tres: %ld %08lx\n",err, res, res);
  return 0;
}

Compile by

$ cc -o bug mjs.c bug.c -ldl

One solution is to implement a strntod, sort of like this something-ish:

double strntod(const char* str, size_t len) {
  char safe_for_strtod[len+1];
  strncpy(safe_for_strtod, str, len);
  safe_for_strtod[len] = 0; // abruptly put an end to any over-reads here
  return strtod(safe_for_strtod, NULL);
}

and use that instead of wherever strtod is used now.

pellepl avatar Jan 18 '20 09:01 pellepl