[libxml2] Fix float casts in xmlXPathSubstringFunction



commit 30a6533e010c740a7281a0c8960b4db9212f5f75
Author: Nick Wellnhofer <wellnhofer aevum de>
Date:   Fri Mar 8 12:15:17 2019 +0100

    Fix float casts in xmlXPathSubstringFunction
    
    Rewrite conversion of double to int in xmlXPathSubstringFunction, adding
    range checks to avoid undefined behavior. Make sure to add start and
    length as floating-point numbers before converting to int. Fix a bug
    when rounding negative start indices.
    
    Remove unneeded calls to xmlXPathIs{Inf,NaN} and rely on IEEE math
    instead. Avoid computing the string length. xmlUTF8Strsub works as
    expected if the length of the requested substring exceeds the input.
    
    Found with libFuzzer and UBSan.

 result/XPath/expr/strings |  8 +++++
 test/XPath/expr/strings   |  2 ++
 xpath.c                   | 80 ++++++++++++++++-------------------------------
 3 files changed, 37 insertions(+), 53 deletions(-)
---
diff --git a/result/XPath/expr/strings b/result/XPath/expr/strings
index 1ae5cc40..4b0125cf 100644
--- a/result/XPath/expr/strings
+++ b/result/XPath/expr/strings
@@ -135,6 +135,14 @@ Object is a string :
 Expression: substring("12345",-1 div 0,5)
 Object is a string : 
 
+========================
+Expression: substring("12345",-0.7,4)
+Object is a string : 12
+
+========================
+Expression: substring("12345",-5000000000,5000000004)
+Object is a string : 123
+
 ========================
 Expression: string-length("")
 Object is a number : 0
diff --git a/test/XPath/expr/strings b/test/XPath/expr/strings
index ba02c794..c741ee25 100644
--- a/test/XPath/expr/strings
+++ b/test/XPath/expr/strings
@@ -32,5 +32,7 @@ substring("12345",3,-1 div 0)
 substring("12345",-42, 1 div 0)
 substring("12345",-1 div 0, 1 div 0)
 substring("12345",-1 div 0,5)
+substring("12345",-0.7,4)
+substring("12345",-5000000000,5000000004)
 string-length("")
 string-length("titi")
diff --git a/xpath.c b/xpath.c
index 5e3bb9ff..72c63382 100644
--- a/xpath.c
+++ b/xpath.c
@@ -9135,8 +9135,7 @@ void
 xmlXPathSubstringFunction(xmlXPathParserContextPtr ctxt, int nargs) {
     xmlXPathObjectPtr str, start, len;
     double le=0, in;
-    int i, l, m;
-    xmlChar *ret;
+    int i = 1, j = INT_MAX;
 
     if (nargs < 2) {
        CHECK_ARITY(2);
@@ -9163,67 +9162,42 @@ xmlXPathSubstringFunction(xmlXPathParserContextPtr ctxt, int nargs) {
     CAST_TO_STRING;
     CHECK_TYPE(XPATH_STRING);
     str = valuePop(ctxt);
-    m = xmlUTF8Strlen((const unsigned char *)str->stringval);
 
-    /*
-     * If last pos not present, calculate last position
-    */
-    if (nargs != 3) {
-       le = (double)m;
-       if (in < 1.0)
-           in = 1.0;
+    if (!(in < INT_MAX)) { /* Logical NOT to handle NaNs */
+        i = INT_MAX;
+    } else if (in >= 1.0) {
+        i = (int)in;
+        if (in - floor(in) >= 0.5)
+            i += 1;
     }
 
-    /* Need to check for the special cases where either
-     * the index is NaN, the length is NaN, or both
-     * arguments are infinity (relying on Inf + -Inf = NaN)
-     */
-    if (!xmlXPathIsInf(in) && !xmlXPathIsNaN(in + le)) {
-        /*
-         * To meet the requirements of the spec, the arguments
-        * must be converted to integer format before
-        * initial index calculations are done
-         *
-         * First we go to integer form, rounding up
-        * and checking for special cases
-         */
-        i = (int) in;
-        if (((double)i)+0.5 <= in) i++;
-
-       if (xmlXPathIsInf(le) == 1) {
-           l = m;
-           if (i < 1)
-               i = 1;
-       }
-       else if (xmlXPathIsInf(le) == -1 || le < 0.0)
-           l = 0;
-       else {
-           l = (int) le;
-           if (((double)l)+0.5 <= le) l++;
-       }
+    if (nargs == 3) {
+        double rin, rle, end;
 
-       /* Now we normalize inidices */
-        i -= 1;
-        l += i;
-        if (i < 0)
-            i = 0;
-        if (l > m)
-            l = m;
+        rin = floor(in);
+        if (in - rin >= 0.5)
+            rin += 1.0;
 
-        /* number of chars to copy */
-        l -= i;
+        rle = floor(le);
+        if (le - rle >= 0.5)
+            rle += 1.0;
 
-        ret = xmlUTF8Strsub(str->stringval, i, l);
-    }
-    else {
-        ret = NULL;
+        end = rin + rle;
+        if (!(end >= 1.0)) { /* Logical NOT to handle NaNs */
+            j = 1;
+        } else if (end < INT_MAX) {
+            j = (int)end;
+        }
     }
-    if (ret == NULL)
-       valuePush(ctxt, xmlXPathCacheNewCString(ctxt->context, ""));
-    else {
+
+    if (i < j) {
+        xmlChar *ret = xmlUTF8Strsub(str->stringval, i - 1, j - i);
        valuePush(ctxt, xmlXPathCacheNewString(ctxt->context, ret));
        xmlFree(ret);
+    } else {
+       valuePush(ctxt, xmlXPathCacheNewCString(ctxt->context, ""));
     }
+
     xmlXPathReleaseObject(ctxt->context, str);
 }
 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]