From f81ffd9a4fe7b173c385c180812b6b3a3e3aa1b8 Mon Sep 17 00:00:00 2001
From: Mark Baker <mark@lange.demon.co.uk>
Date: Mon, 8 Mar 2021 12:54:06 +0100
Subject: [PATCH] Additional argument validation for LEFT(), MID() and RIGHT()
 text functions (#1909)

* Additional argument validation for LEFT(), MID() and RIGHT() text functions
---
 CHANGELOG.md                                |  1 +
 src/PhpSpreadsheet/Calculation/TextData.php | 10 +++-------
 tests/data/Calculation/TextData/LEFT.php    | 10 ++++++++++
 tests/data/Calculation/TextData/MID.php     | 14 +++++++++++++-
 tests/data/Calculation/TextData/RIGHT.php   | 10 ++++++++++
 5 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index b59bbe9d..2474e26d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -26,6 +26,7 @@ and this project adheres to [Semantic Versioning](https://semver.org).
 ### Fixed
 
 - Fixed issue with Xlsx@listWorksheetInfo not returning any data
+- Fixed invalid arguments triggering mb_substr() error in LEFT(), MID() and RIGHT() text functions. [Issue #640](https://github.com/PHPOffice/PhpSpreadsheet/issues/640)
 
 ## 1.17.1 - 2021-03-01
 
diff --git a/src/PhpSpreadsheet/Calculation/TextData.php b/src/PhpSpreadsheet/Calculation/TextData.php
index cea15fdc..b886ce08 100644
--- a/src/PhpSpreadsheet/Calculation/TextData.php
+++ b/src/PhpSpreadsheet/Calculation/TextData.php
@@ -299,7 +299,7 @@ class TextData
         $value = Functions::flattenSingleValue($value);
         $chars = Functions::flattenSingleValue($chars);
 
-        if ($chars < 0) {
+        if (!is_numeric($chars) || $chars < 0) {
             return Functions::VALUE();
         }
 
@@ -325,7 +325,7 @@ class TextData
         $start = Functions::flattenSingleValue($start);
         $chars = Functions::flattenSingleValue($chars);
 
-        if (($start < 1) || ($chars < 0)) {
+        if (!is_numeric($start) || $start < 1 || !is_numeric($chars) || $chars < 0) {
             return Functions::VALUE();
         }
 
@@ -333,10 +333,6 @@ class TextData
             $value = ($value) ? Calculation::getTRUE() : Calculation::getFALSE();
         }
 
-        if (empty($chars)) {
-            return '';
-        }
-
         return mb_substr($value, --$start, $chars, 'UTF-8');
     }
 
@@ -353,7 +349,7 @@ class TextData
         $value = Functions::flattenSingleValue($value);
         $chars = Functions::flattenSingleValue($chars);
 
-        if ($chars < 0) {
+        if (!is_numeric($chars) || $chars < 0) {
             return Functions::VALUE();
         }
 
diff --git a/tests/data/Calculation/TextData/LEFT.php b/tests/data/Calculation/TextData/LEFT.php
index 914d16be..96702f6b 100644
--- a/tests/data/Calculation/TextData/LEFT.php
+++ b/tests/data/Calculation/TextData/LEFT.php
@@ -16,6 +16,16 @@ return [
         'QWERTYUIOP',
         -1,
     ],
+    [
+        '#VALUE!',
+        'QWERTYUIOP',
+        'NaN',
+    ],
+    [
+        '#VALUE!',
+        'QWERTYUIOP',
+        null,
+    ],
     [
         'ABC',
         'ABCDEFGHI',
diff --git a/tests/data/Calculation/TextData/MID.php b/tests/data/Calculation/TextData/MID.php
index 2a59373b..71d90e8b 100644
--- a/tests/data/Calculation/TextData/MID.php
+++ b/tests/data/Calculation/TextData/MID.php
@@ -26,7 +26,19 @@ return [
         -1,
     ],
     [
-        '',
+        '#VALUE!',
+        'QWERTYUIOP',
+        'NaN',
+        1,
+    ],
+    [
+        '#VALUE!',
+        'QWERTYUIOP',
+        2,
+        'NaN',
+    ],
+    [
+        '#VALUE!',
         'QWERTYUIOP',
         5,
     ],
diff --git a/tests/data/Calculation/TextData/RIGHT.php b/tests/data/Calculation/TextData/RIGHT.php
index 78ea6a69..95dfe96e 100644
--- a/tests/data/Calculation/TextData/RIGHT.php
+++ b/tests/data/Calculation/TextData/RIGHT.php
@@ -16,6 +16,16 @@ return [
         'QWERTYUIOP',
         -1,
     ],
+    [
+        '#VALUE!',
+        'QWERTYUIOP',
+        'NaN',
+    ],
+    [
+        '#VALUE!',
+        'QWERTYUIOP',
+        null,
+    ],
     [
         'GHI',
         'ABCDEFGHI',