PFW: overflow not detected for int parameters

BZ: 26285

The following errors were not detected by the PFW when setting parameters
of type (U)INT8, (U)INT16, (U)INT32:
- When setting a value out of the int32 range (ex: 999999999999999), the
strtol/strtoul functions return the value -1 which was then assumed correct
by the PFW. Now the errno value is checked to ensure that no range error
was encountered by strtol/strtoul.
- When the input string does not contain any digits, the strtol/strtoul
functions return 0 which was assumed correct by the PFW. Now the endptr
argument is checked to make sure that at least a part of the string was
parsed.

In any case an error message is displayed and the original value is not
updated.

Made the change compliant to 64-bit OSes.

Applied the same corrections to Enum and FixedPoint types.

Change-Id: I135538def791208a6eb6143444a3fc30337137e1
Orig-Change-Id: I1519dbf798228a9be579aaf612f456d5eb1b41b5
Signed-off-by: Frederic Boisnard <fredericx.boisnard@intel.com>
Reviewed-on: http://android.intel.com:8080/55443
Reviewed-by: Mendi, EduardoX <eduardox.mendi@intel.com>
Tested-by: Mendi, EduardoX <eduardox.mendi@intel.com>
Reviewed-by: buildbot <buildbot@intel.com>
Tested-by: buildbot <buildbot@intel.com>

1 files changed, 71 insertions, 39 deletions

diff --git a/parameter/FixedPointParameterType.cpp b/parameter/FixedPointParameterType.cpp
index 63d4d4c..0580c55 100644
--- a/parameter/FixedPointParameterType.cpp
+++ b/parameter/FixedPointParameterType.cpp
@@ -37,6 +37,7 @@
 #include "Parameter.h"
 #include "ParameterAccessContext.h"
 #include "ConfigurationAccessContext.h"
+#include <errno.h>
 
 #define base CParameterType
 
@@ -122,36 +123,57 @@ bool CFixedPointParameterType::toBlackboard(const string& strValue, uint32_t& ui
         return false;
     }
 
-    int32_t iData;
+    int64_t iData;
 
     if (parameterAccessContext.valueSpaceIsRaw()) {
+        errno = 0;
+        char *pcStrEnd;
 
         // Get data in integer form
-        iData = strtol(strValue.c_str(), NULL, 0);
+        iData = strtoll(strValue.c_str(), &pcStrEnd, 0);
 
-        if (bValueProvidedAsHexa && isEncodable(iData)) {
+        // Conversion error when the input string does not contain any digit or the number is out of range
+        bool bConversionSucceeded = !errno && (strValue.c_str() != pcStrEnd);
+
+        if (!bConversionSucceeded || !isEncodable((uint64_t)iData, !bValueProvidedAsHexa)) {
+
+            // Illegal value provided
+            parameterAccessContext.setError(getOutOfRangeError(strValue, parameterAccessContext.valueSpaceIsRaw(), bValueProvidedAsHexa));
+
+            return false;
+        }
+        if (bValueProvidedAsHexa) {
 
             // Sign extend
             signExtend(iData);
         }
 
     } else {
-        double dData = strtod(strValue.c_str(), NULL);
+        errno = 0;
+        char *pcStrEnd;
 
-        // Do the conversion
-        iData = asInteger(dData);
-    }
+        double dData = strtod(strValue.c_str(), &pcStrEnd);
 
-    // Check integrity
-    if (!isConsistent(iData)) {
+        // Conversion error when the input string does not contain any digit or the number is out of range (int32_t type)
+        bool bConversionSucceeded = !errno && (strValue.c_str() != pcStrEnd);
 
-        // Illegal value provided
-        parameterAccessContext.setError(getOutOfRangeError(strValue, parameterAccessContext.valueSpaceIsRaw(), bValueProvidedAsHexa));
+        // Check encodability
+        if (!bConversionSucceeded || !checkValueAgainstRange(dData)) {
 
-        return false;
+            // Illegal value provided
+            parameterAccessContext.setError(getOutOfRangeError(strValue, parameterAccessContext.valueSpaceIsRaw(), bValueProvidedAsHexa));
+
+            return false;
+        }
+
+        // Do the conversion
+        iData = asInteger(dData);
     }
 
-    uiValue = iData;
+    // check that the data is encodable and can be safely written to the blackboard
+    assert(isEncodable((unsigned long int)iData, true));
+
+    uiValue = (uint32_t)iData;
 
     return true;
 }
@@ -160,8 +182,8 @@ bool CFixedPointParameterType::fromBlackboard(string& strValue, const uint32_t&
 {
     int32_t iData = uiValue;
 
-    // Check consistency
-    assert(isEncodable(iData));
+    // Check encodability
+    assert(isEncodable((uint32_t)iData, false));
 
     // Format
     ostringstream strStream;
@@ -202,11 +224,8 @@ bool CFixedPointParameterType::fromBlackboard(string& strValue, const uint32_t&
 // Value access
 bool CFixedPointParameterType::toBlackboard(double dUserValue, uint32_t& uiValue, CParameterAccessContext& parameterAccessContext) const
 {
-    // Do the conversion
-    int32_t iData = asInteger(dUserValue);
-
-    // Check integrity
-    if (!isConsistent(iData)) {
+    // Check that the value is within the allowed range for this type
+    if (!checkValueAgainstRange(dUserValue)) {
 
         // Illegal value provided
         parameterAccessContext.setError("Value out of range");
@@ -214,6 +233,12 @@ bool CFixedPointParameterType::toBlackboard(double dUserValue, uint32_t& uiValue
         return false;
     }
 
+    // Do the conversion
+    int32_t iData = asInteger(dUserValue);
+
+    // Check integrity
+    assert(isEncodable((uint32_t)iData, true));
+
     uiValue = iData;
 
     return true;
@@ -225,8 +250,8 @@ bool CFixedPointParameterType::fromBlackboard(double& dUserValue, uint32_t uiVal
 
     int32_t iData = uiValue;
 
-    // Check consistency
-    assert(isEncodable(iData));
+    // Check unsigned value is encodable
+    assert(isEncodable(uiValue, false));
 
     // Sign extend
     signExtend(iData);
@@ -242,12 +267,17 @@ uint32_t CFixedPointParameterType::getUtilSizeInBits() const
     return _uiIntegral + _uiFractional + 1;
 }
 
+// Compute the range for the type (minimum and maximum values)
+void CFixedPointParameterType::getRange(double& dMin, double& dMax) const
+{
+    dMax = (double)((1UL << (_uiIntegral + _uiFractional)) - 1) / (1UL << _uiFractional);
+    dMin = -(double)(1UL << (_uiIntegral + _uiFractional)) / (1UL << _uiFractional);
+}
+
 // Out of range error
 string CFixedPointParameterType::getOutOfRangeError(const string& strValue, bool bRawValueSpace, bool bHexaValue) const
 {
-    // Min/Max computation
-    int32_t iMax = (1L << (getSize() * 8 - 1)) - 1;
-    int32_t iMin = -iMax - 1;
+
 
     ostringstream strStream;
 
@@ -255,9 +285,18 @@ string CFixedPointParameterType::getOutOfRangeError(const string& strValue, bool
 
     if (!bRawValueSpace) {
 
-        strStream << "real range [" << (double)iMin / (1UL << _uiFractional) << ", "<< (double)iMax / (1UL << _uiFractional) << "]";
+        // Min/Max computation
+        double dMin = 0;
+        double dMax = 0;
+        getRange(dMin, dMax);
+
+        strStream << "real range [" << dMin << ", "<< dMax << "]";
     } else {
 
+        // Min/Max computation
+        int32_t iMax = (1L << (getSize() * 8 - 1)) - 1;
+        int32_t iMin = -iMax - 1;
+
         strStream << "raw range [";
 
         if (bHexaValue) {
@@ -279,21 +318,14 @@ string CFixedPointParameterType::getOutOfRangeError(const string& strValue, bool
     return strStream.str();
 }
 
-// Check data is consistent with available range, with respect to its sign
-bool CFixedPointParameterType::isConsistent(uint32_t uiData) const
+// Check that the value is within available range for this type
+bool CFixedPointParameterType::checkValueAgainstRange(double dValue) const
 {
-    uint32_t uiShift = getSize() * 8;
-
-    if (uiShift == 8 * sizeof(uiData)) {
-        // Prevent inappropriate shifts
-        return true;
-    }
-
-    // Negative value?
-    bool bIsValueExpectedNegative = (uiData & (1 << (uiShift - 1))) != 0;
+    double dMin = 0;
+    double dMax = 0;
+    getRange(dMin, dMax);
 
-    // Check high bits are clean
-    return bIsValueExpectedNegative ? !(~uiData >> uiShift) : !(uiData >> uiShift);
+    return (dValue <= dMax) && (dValue >= dMin);
 }
 
 // Data conversion