fixing a critical bug in UA_Double_decodeBinary

src/server/ua_server_binary.c

@@ -126,18 +126,6 @@ static void processOPN(UA_Connection *connection, UA_Server *server, const UA_By
     connection->write(connection, (UA_ByteStringArray){ .stringsSize = 1, .strings = &resp_msg });
 }
 
-static void handle_diagnostics(const UA_RequestHeader *p, UA_ResponseHeader *r){
-	// TODO do it right, we will probably need to to it inside of the service
-	if(p->returnDiagnostics > 0){
-		r->serviceDiagnostics.hasSymbolicId = UA_TRUE;
-		r->serviceDiagnostics.symbolicId = 0;
-		r->stringTableSize = 1;
-		UA_String* namespaceUri = UA_String_new();
-		UA_String_copycstring("TBD", namespaceUri);
-		r->stringTable = namespaceUri;
-	}
-}
-
 static void init_response_header(const UA_RequestHeader *p, UA_ResponseHeader *r) {
     r->requestHandle = p->requestHandle;
     r->serviceResult = UA_STATUSCODE_GOOD;
@@ -163,7 +151,6 @@ static void init_response_header(const UA_RequestHeader *p, UA_ResponseHeader *r
             return;                                                     \
         UA_##TYPE##Response_init(&r);                                   \
         init_response_header(&p.requestHeader, &r.responseHeader);      \
-        handle_diagnostics(&p.requestHeader, &r.responseHeader);											\
         Service_##TYPE(server, clientSession, &p, &r);                  \
         ALLOC_MESSAGE(message, UA_##TYPE##Response_calcSizeBinary(&r)); \
         UA_##TYPE##Response_encodeBinary(&r, message, &sendOffset);     \

src/server/ua_services_attribute.c

@@ -289,13 +289,8 @@ void Service_Read(UA_Server *server, UA_Session *session, const UA_ReadRequest *
 
     response->resultsSize = size;
 
-    //a bit ugly, but probably almost never called
     if(request->maxAge < 0) {
-        for(size_t i = 0;i < size;i++) {
-        	response->results[i].status = UA_STATUSCODE_BADMAXAGEINVALID;
-        	handleServerTimestamps(request->timestampsToReturn, &response->results[i]);
-        	handleSourceTimestamps(request->timestampsToReturn, &response->results[i]);
-        }
+    	response->responseHeader.serviceResult = UA_STATUSCODE_BADMAXAGEINVALID;
         return;
     }
 

src/ua_types_encoding_binary.c

@@ -155,7 +155,7 @@ UA_StatusCode UA_Float_decodeBinary(UA_ByteString const *src, size_t *offset, UA
     biasedExponent |= (UA_UInt32)(src->data[*offset+3] & 0x7F) <<  1;                   // bits 24-30
     sign = ( src->data[*offset+ 3] & 0x80 ) ? -1.0 : 1.0;                               // bit 31
     if(biasedExponent >= 127)
-        *dst = (UA_Float)sign * (1 << (biasedExponent-127)) * (1.0 + mantissa / 128.0 );
+        *dst = (UA_Float)sign * (1ULL << (biasedExponent-127)) * (1.0 + mantissa / 128.0 );
     else
         *dst = (UA_Float)sign * 2.0 * (1.0 + mantissa / 128.0 ) / ((UA_Float)(biasedExponent-127));
     *offset += 4;
@@ -171,6 +171,7 @@ UA_TYPE_CALCSIZEBINARY_MEMSIZE(UA_Double)
 // FIXME: Implement NaN, Inf and Zero(s)
 UA_Byte UA_DOUBLE_ZERO[] = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 };
 UA_StatusCode UA_Double_decodeBinary(UA_ByteString const *src, size_t *offset, UA_Double * dst) {
+#include "stdio.h"
     if(*offset + sizeof(UA_Double) > (UA_UInt32)src->length )
         return UA_STATUSCODE_BADDECODINGERROR;
     UA_Double sign;
@@ -189,7 +190,7 @@ UA_StatusCode UA_Double_decodeBinary(UA_ByteString const *src, size_t *offset, U
     biasedExponent |= ((UA_UInt32)(src->data[*offset+7] & 0x7F)) <<  4; // bits 56-62
     sign = ( src->data[*offset+7] & 0x80 ) ? -1.0 : 1.0; // bit 63
     if(biasedExponent >= 1023)
-        *dst = (UA_Double)sign * (1 << (biasedExponent-1023)) * (1.0 + mantissa / 8.0 );
+        *dst = (UA_Double)sign * (1ULL << (biasedExponent-1023)) * (1.0 + mantissa / 8.0 );
     else
         *dst = (UA_Double)sign * 2.0 *
             (1.0 + mantissa / 8.0 ) / ((UA_Double)(biasedExponent-1023));

tests/check_builtin.c

@@ -564,6 +564,22 @@ START_TEST(UA_Double_decodeShallGiveMinusTwo) {
 }
 END_TEST
 
+START_TEST(UA_Double_decodeShallGive2147483648) {
+	// given
+	size_t pos = 0;
+	UA_Byte data[] = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xE0, 0x41 }; //2147483648
+	UA_ByteString src = { 8, data }; // 1
+	UA_Double dst;
+	// when
+	UA_StatusCode retval = UA_Double_decodeBinary(&src, &pos, &dst);
+	// then
+	ck_assert_int_eq(retval, UA_STATUSCODE_GOOD);
+	ck_assert_int_eq(pos, 8);
+	ck_assert(2147483647.9999999 < dst);
+	ck_assert(dst < 2147483648.00000001);
+}
+END_TEST
+
 START_TEST(UA_String_decodeShallAllocateMemoryAndCopyString) {
 	// given
 	size_t pos = 0;
@@ -1689,6 +1705,7 @@ static Suite *testSuite_builtin(void) {
 	tcase_add_test(tc_decode, UA_Double_decodeShallGiveOne);
 	tcase_add_test(tc_decode, UA_Double_decodeShallGiveZero);
 	tcase_add_test(tc_decode, UA_Double_decodeShallGiveMinusTwo);
+	tcase_add_test(tc_decode, UA_Double_decodeShallGive2147483648);
 	tcase_add_test(tc_decode, UA_Byte_encode_test);
 	tcase_add_test(tc_decode, UA_String_decodeShallAllocateMemoryAndCopyString);
 	tcase_add_test(tc_decode, UA_String_decodeWithNegativeSizeShallNotAllocateMemoryAndNullPtr);
@@ -1752,7 +1769,7 @@ int main(void) {
 
 	s  = testSuite_builtin();
 	sr = srunner_create(s);
-	//srunner_set_fork_status(sr, CK_NOFORK);
+	srunner_set_fork_status(sr, CK_NOFORK);
 	srunner_run_all(sr, CK_NORMAL);
 	number_failed += srunner_ntests_failed(sr);
 	srunner_free(sr);