CVE-2018-11592: Out-of-bounds Read (CWE-125)
value: Espruino before 1.98 allows attackers to cause a denial of service (application crash) with a user crafted input file via an Out-of-bounds Read during syntax parsing in which certain height validation is missing in libs/graphics/jswrap_graphics.c.
CWE description: The product reads data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 5.5
Published: 2018-05-31T16:29Z
Repository: https://github.com/espruino/Espruino
Language: C
Files changed: ChangeLog, libs/graphics/jswrap_graphics.c, libs/graphics/lcd_arraybuffer.c
Commit date: 2018-05-18 10:02:14 +0100
Commit message: Add height check for Graphics.createArrayBuffer(...vertical_byte:true) (fix #1421)

Diff: diff --git a/ChangeLog b/ChangeLog
index dd990837..fcadd4b6 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -19,10 +19,11 @@
             nRF52: Added `NRF.setAddress` to allow the MAC address to be changed
             Added Graphics.setFontAlign for font alignment and rotation
             Make software I2C bitrate and waveform more accurate
             Move default I2C bitrate to 100kbit/sec
             Linux: don't create a espruino.flash file if we're not writing to flash
+            Add height check for Graphics.createArrayBuffer(...vertical_byte:true) (fix #1421)
 
      1v97 : nRF52: fix NRF.on('connect',...) issue
             STM32: Fix setDeviceClockCmd error for USB.setConsole()
             nRF5x: Fix getPinMode, which fixes SW I2C after save()
             Thingy52: Don't report contents of 'Thingy' in 'dump()'
diff --git a/libs/graphics/jswrap_graphics.c b/libs/graphics/jswrap_graphics.c
index 04505ce4..2704c410 100644
--- a/libs/graphics/jswrap_graphics.c
+++ b/libs/graphics/jswrap_graphics.c
@@ -127,12 +127,18 @@ JsVar *jswrap_graphics_createArrayBuffer(int width, int height, int bpp, JsVar *
     if (jsvGetBoolAndUnLock(jsvObjectGetChild(options, "msb", 0)))
       gfx.data.flags = (JsGraphicsFlags)(gfx.data.flags | JSGRAPHICSFLAGS_ARRAYBUFFER_MSB);
     if (jsvGetBoolAndUnLock(jsvObjectGetChild(options, "vertical_byte", 0))) {
       if (gfx.data.bpp==1)
         gfx.data.flags = (JsGraphicsFlags)(gfx.data.flags | JSGRAPHICSFLAGS_ARRAYBUFFER_VERTICAL_BYTE);
-      else
-        jsWarn("vertical_byte only works for 1bpp ArrayBuffers\n");
+      else {
+        jsExceptionHere(JSET_ERROR, "vertical_byte only works for 1bpp ArrayBuffers\n");
+        return 0;
+      }
+      if (gfx.data.height&7) {
+        jsExceptionHere(JSET_ERROR, "height must be a multiple of 8 when using vertical_byte\n");
+        return 0;
+      }
     }
     JsVar *colorv = jsvObjectGetChild(options, "color_order", 0);
     if (colorv) {
       if (jsvIsStringEqual(colorv, "rgb")) ; // The default
       else if (!jsvIsStringEqual(colorv, "brg"))
diff --git a/libs/graphics/lcd_arraybuffer.c b/libs/graphics/lcd_arraybuffer.c
index ec933b6c..3a299565 100644
--- a/libs/graphics/lcd_arraybuffer.c
+++ b/libs/graphics/lcd_arraybuffer.c
@@ -159,10 +159,11 @@ void lcdSetPixels_ArrayBuffer_flat(JsGraphics *gfx, short x, short y, short pixe
         }
       }
       unsigned int mask = (unsigned int)(1<<gfx->data.bpp)-1;
       unsigned int existing = (unsigned int)*ptr;
       unsigned int bitIdx = (gfx->data.flags & JSGRAPHICSFLAGS_ARRAYBUFFER_MSB) ? 8-(idx+gfx->data.bpp) : idx;
+      assert(ptr>=gfx->backendData && ptr<((char*)gfx->backendData + graphicsGetMemoryRequired(gfx)));
       *ptr = (char)((existing&~(mask<<bitIdx)) | ((col&mask)<<bitIdx));
       if (gfx->data.flags & JSGRAPHICSFLAGS_ARRAYBUFFER_VERTICAL_BYTE) {
         ptr++;
       } else {
         idx += gfx->data.bpp;

Vulnerable code:       else
        jsWarn("vertical_byte only works for 1bpp ArrayBuffers\n");
Fixed code:             Add height check for Graphics.createArrayBuffer(...vertical_byte:true) (fix #1421)
      else {
        jsExceptionHere(JSET_ERROR, "vertical_byte only works for 1bpp ArrayBuffers\n");
        return 0;
      }
      if (gfx.data.height&7) {
        jsExceptionHere(JSET_ERROR, "height must be a multiple of 8 when using vertical_byte\n");
        return 0;
      }
      assert(ptr>=gfx->backendData && ptr<((char*)gfx->backendData + graphicsGetMemoryRequired(gfx)));