From 6484bf5f3f33a62670a29bf9a1f08bf62b64f145 Mon Sep 17 00:00:00 2001
From: Caolán McNamara <caolanm@redhat.com>
Date: Wed, 29 Oct 2014 11:04:40 +0000
Subject: untaint registry data

by using a byte-swapping pattern that coverity doesn't detect as such

tested as a scratch coverity attempt with a smaller project which
has a far higher allocation of coverity attempts per week :-)

unsigned int readTaintedUINT32(const char* buffer)
{
    unsigned int v = (
            (buffer[0] << 24) |
            (buffer[1] << 16) |
            (buffer[2] << 8)  |
            (buffer[3] << 0)
        );

    return v;
}

unsigned int readUntaintedUINT32(const char* p)
{
    unsigned int v = *p++; v <<= 8;
    v |= *p++; v <<= 8;
    v |= *p++; v <<= 8;
    return v | *p;
}

void foo(char *buffer)
{
    char *pOne = new char[readTaintedUINT32(buffer)];
	// ^ coverity only reports this
    delete [] pOne;

    char *pTwo = new char[readUntaintedUINT32(buffer)];
	// ^ and not this
    delete [] pTwo;
}

should silence

coverity#1213371 Untrusted value as argument
coverity#1213372 Untrusted value as argument
coverity#1213373 Use of untrusted scalar value
coverity#1213374 Use of untrusted scalar value
coverity#1213376 Untrusted loop bound
coverity#1213388 Use of untrusted scalar value
coverity#1213389 Use of untrusted scalar value
coverity#1213390 Use of untrusted scalar value
coverity#1213423 Untrusted value as argument
coverity#1213424 Untrusted value as argument
coverity#1213425 Untrusted value as argument
coverity#1213432 Untrusted value as argument
coverity#1215304 Untrusted loop bound

Change-Id: Ib8c7fc9a8e8b36ca227c76577d991c10df7dcd5a
---
 registry/source/reflcnst.hxx | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

(limited to 'registry')

diff --git a/registry/source/reflcnst.hxx b/registry/source/reflcnst.hxx
index b76475e0c75f..e9e39449a4b1 100644
--- a/registry/source/reflcnst.hxx
+++ b/registry/source/reflcnst.hxx
@@ -138,8 +138,12 @@ inline sal_uInt32 writeUINT16(sal_uInt8* buffer, sal_uInt16 v)
 
 inline sal_uInt32 readUINT16(const sal_uInt8* buffer, sal_uInt16& v)
 {
-    v = ((buffer[0] << 8) | (buffer[1] << 0));
-
+    //This is untainted data which comes from a controlled source
+    //so, using a byte-swapping pattern which coverity doesn't
+    //detect as such
+    //http://security.coverity.com/blog/2014/Apr/on-detecting-heartbleed-with-static-analysis.html
+    v = *buffer++; v <<= 8;
+    v |= *buffer;
     return sizeof(sal_uInt16);
 }
 
@@ -177,13 +181,14 @@ inline sal_uInt32 writeUINT32(sal_uInt8* buffer, sal_uInt32 v)
 
 inline sal_uInt32 readUINT32(const sal_uInt8* buffer, sal_uInt32& v)
 {
-    v = (
-            (buffer[0] << 24) |
-            (buffer[1] << 16) |
-            (buffer[2] << 8)  |
-            (buffer[3] << 0)
-        );
-
+    //This is untainted data which comes from a controlled source
+    //so, using a byte-swapping pattern which coverity doesn't
+    //detect as such
+    //http://security.coverity.com/blog/2014/Apr/on-detecting-heartbleed-with-static-analysis.html
+    v = *buffer++; v <<= 8;
+    v |= *buffer++; v <<= 8;
+    v |= *buffer++; v <<= 8;
+    v |= *buffer;
     return sizeof(sal_uInt32);
 }
 
-- 
cgit