CVE-2023-27772: Improper Check for Unusual or Exceptional Conditions (CWE-754)
value: libiec61850 v1.5.1 was discovered to contain a segmentation violation via the function ControlObjectClient_setOrigin() at /client/client_control.c.
CWE description: The product does not check or incorrectly checks for unusual or exceptional conditions that are not expected to occur frequently during day to day operation of the product.
CVSS v3 base score: 7.5
Published: 2023-04-13T18:15Z
Repository: https://github.com/mz-automation/libiec61850
Language: C
Files changed: examples/iec61850_client_example_control/client_example_control.c
Commit date: 2023-02-24 11:07:09 +0000
Commit message: - added missing checks in iec61850_client_example_control (#442)

Diff: diff --git a/examples/iec61850_client_example_control/client_example_control.c b/examples/iec61850_client_example_control/client_example_control.c
index 33da635a..37d6b6bc 100644
--- a/examples/iec61850_client_example_control/client_example_control.c
+++ b/examples/iec61850_client_example_control/client_example_control.c
@@ -43,171 +43,200 @@ int main(int argc, char** argv) {
 
     IedConnection con = IedConnection_create();
 
     IedConnection_connect(con, &error, hostname, tcpPort);
 
-    if (error == IED_ERROR_OK) {
-
+    if (error == IED_ERROR_OK)
+    {
+        MmsValue* ctlVal = NULL;
+        MmsValue* stVal = NULL;
 
         /************************
          * Direct control
          ***********************/
 
         ControlObjectClient control
             = ControlObjectClient_create("simpleIOGenericIO/GGIO1.SPCSO1", con);
 
-        MmsValue* ctlVal = MmsValue_newBoolean(true);
+        if (control)
+        {
+            ctlVal = MmsValue_newBoolean(true);
 
-        ControlObjectClient_setOrigin(control, NULL, 3);
+            ControlObjectClient_setOrigin(control, NULL, 3);
 
-        if (ControlObjectClient_operate(control, ctlVal, 0 /* operate now */)) {
-            printf("simpleIOGenericIO/GGIO1.SPCSO1 operated successfully\n");
-        }
-        else {
-            printf("failed to operate simpleIOGenericIO/GGIO1.SPCSO1\n");
-        }
+            if (ControlObjectClient_operate(control, ctlVal, 0 /* operate now */)) {
+                printf("simpleIOGenericIO/GGIO1.SPCSO1 operated successfully\n");
+            }
+            else {
+                printf("failed to operate simpleIOGenericIO/GGIO1.SPCSO1\n");
+            }
+
+            MmsValue_delete(ctlVal);
 
-        MmsValue_delete(ctlVal);
+            ControlObjectClient_destroy(control);
 
-        ControlObjectClient_destroy(control);
+            /* Check if status value has changed */
 
-        /* Check if status value has changed */
+            stVal = IedConnection_readObject(con, &error, "simpleIOGenericIO/GGIO1.SPCSO1.stVal", IEC61850_FC_ST);
 
-        MmsValue* stVal = IedConnection_readObject(con, &error, "simpleIOGenericIO/GGIO1.SPCSO1.stVal", IEC61850_FC_ST);
+            if (error == IED_ERROR_OK) {
+                bool state = MmsValue_getBoolean(stVal);
+                MmsValue_delete(stVal);
 
-        if (error == IED_ERROR_OK) {
-            bool state = MmsValue_getBoolean(stVal);
-            MmsValue_delete(stVal);
+                printf("New status of simpleIOGenericIO/GGIO1.SPCSO1.stVal: %i\n", state);
+            }
+            else {
+                printf("Reading status for simpleIOGenericIO/GGIO1.SPCSO1 failed!\n");
+            }
 
-            printf("New status of simpleIOGenericIO/GGIO1.SPCSO1.stVal: %i\n", state);
         }
         else {
-            printf("Reading status for simpleIOGenericIO/GGIO1.SPCSO1 failed!\n");
+            printf("Control object simpleIOGenericIO/GGIO1.SPCSO1 not found in server\n");
         }
 
-
         /************************
          * Select before operate
          ***********************/
 
         control = ControlObjectClient_create("simpleIOGenericIO/GGIO1.SPCSO2", con);
 
-        if (ControlObjectClient_select(control)) {
+        if (control)
+        {
+            if (ControlObjectClient_select(control)) {
 
-            ctlVal = MmsValue_newBoolean(true);
+                ctlVal = MmsValue_newBoolean(true);
 
-            if (ControlObjectClient_operate(control, ctlVal, 0 /* operate now */)) {
-                printf("simpleIOGenericIO/GGIO1.SPCSO2 operated successfully\n");
+                if (ControlObjectClient_operate(control, ctlVal, 0 /* operate now */)) {
+                    printf("simpleIOGenericIO/GGIO1.SPCSO2 operated successfully\n");
+                }
+                else {
+                    printf("failed to operate simpleIOGenericIO/GGIO1.SPCSO2!\n");
+                }
+
+                MmsValue_delete(ctlVal);
             }
             else {
-                printf("failed to operate simpleIOGenericIO/GGIO1.SPCSO2!\n");
+                printf("failed to select simpleIOGenericIO/GGIO1.SPCSO2!\n");
             }
 
-            MmsValue_delete(ctlVal);
+            ControlObjectClient_destroy(control);
         }
         else {
-            printf("failed to select simpleIOGenericIO/GGIO1.SPCSO2!\n");
+            printf("Control object simpleIOGenericIO/GGIO1.SPCSO2 not found in server\n");
         }
 
-        ControlObjectClient_destroy(control);
-
-
         /****************************************
          * Direct control with enhanced security
          ****************************************/
 
         control = ControlObjectClient_create("simpleIOGenericIO/GGIO1.SPCSO3", con);
 
-        ControlObjectClient_setCommandTerminationHandler(control, commandTerminationHandler, NULL);
+        if (control)
+        {
+            ControlObjectClient_setCommandTerminationHandler(control, commandTerminationHandler, NULL);
 
-        ctlVal = MmsValue_newBoolean(true);
+            ctlVal = MmsValue_newBoolean(true);
 
-        if (ControlObjectClient_operate(control, ctlVal, 0 /* operate now */)) {
-            printf("simpleIOGenericIO/GGIO1.SPCSO3 operated successfully\n");
-        }
-        else {
-            printf("failed to operate simpleIOGenericIO/GGIO1.SPCSO3\n");
-        }
+            if (ControlObjectClient_operate(control, ctlVal, 0 /* operate now */)) {
+                printf("simpleIOGenericIO/GGIO1.SPCSO3 operated successfully\n");
+            }
+            else {
+                printf("failed to operate simpleIOGenericIO/GGIO1.SPCSO3\n");
+            }
 
-        MmsValue_delete(ctlVal);
+            MmsValue_delete(ctlVal);
 
-        /* Wait for command termination message */
-        Thread_sleep(1000);
+            /* Wait for command termination message */
+            Thread_sleep(1000);
 
-        ControlObjectClient_destroy(control);
+            ControlObjectClient_destroy(control);
 
-        /* Check if status value has changed */
+            /* Check if status value has changed */
 
-       stVal = IedConnection_readObject(con, &error, "simpleIOGenericIO/GGIO1.SPCSO3.stVal", IEC61850_FC_ST);
+            stVal = IedConnection_readObject(con, &error, "simpleIOGenericIO/GGIO1.SPCSO3.stVal", IEC61850_FC_ST);
 
-        if (error == IED_ERROR_OK) {
-            bool state = MmsValue_getBoolean(stVal);
+            if (error == IED_ERROR_OK) {
+                bool state = MmsValue_getBoolean(stVal);
 
-            printf("New status of simpleIOGenericIO/GGIO1.SPCSO3.stVal: %i\n", state);
+                printf("New status of simpleIOGenericIO/GGIO1.SPCSO3.stVal: %i\n", state);
 
-            MmsValue_delete(stVal);
+                MmsValue_delete(stVal);
+            }
+            else {
+                printf("Reading status for simpleIOGenericIO/GGIO1.SPCSO3 failed!\n");
+            }
         }
         else {
-            printf("Reading status for simpleIOGenericIO/GGIO1.SPCSO3 failed!\n");
+            printf("Control object simpleIOGenericIO/GGIO1.SPCSO3 not found in server\n");
         }
 
         /***********************************************
          * Select before operate with enhanced security
          ***********************************************/
 
         control = ControlObjectClient_create("simpleIOGenericIO/GGIO1.SPCSO4", con);
 
-        ControlObjectClient_setCommandTerminationHandler(control, commandTerminationHandler, NULL);
+        if (control)
+        {
+            ControlObjectClient_setCommandTerminationHandler(control, commandTerminationHandler, NULL);
 
-        ctlVal = MmsValue_newBoolean(true);
+            ctlVal = MmsValue_newBoolean(true);
 
-        if (ControlObjectClient_selectWithValue(control, ctlVal)) {
+            if (ControlObjectClient_selectWithValue(control, ctlVal)) {
+
+                if (ControlObjectClient_operate(control, ctlVal, 0 /* operate now */)) {
+                    printf("simpleIOGenericIO/GGIO1.SPCSO4 operated successfully\n");
+                }
+                else {
+                    printf("failed to operate simpleIOGenericIO/GGIO1.SPCSO4!\n");
+                }
 
-            if (ControlObjectClient_operate(control, ctlVal, 0 /* operate now */)) {
-                printf("simpleIOGenericIO/GGIO1.SPCSO4 operated successfully\n");
             }
             else {
-                printf("failed to operate simpleIOGenericIO/GGIO1.SPCSO4!\n");
+                printf("failed to select simpleIOGenericIO/GGIO1.SPCSO4!\n");
             }
 
+            MmsValue_delete(ctlVal);
+
+            /* Wait for command termination message */
+            Thread_sleep(1000);
+
+            ControlObjectClient_destroy(control);
         }
         else {
-            printf("failed to select simpleIOGenericIO/GGIO1.SPCSO4!\n");
+            printf("Control object simpleIOGenericIO/GGIO1.SPCSO4 not found in server\n");
         }
 
-        MmsValue_delete(ctlVal);
-
-        /* Wait for command termination message */
-        Thread_sleep(1000);
-
-        ControlObjectClient_destroy(control);
-
-
         /*********************************************************************
          * Direct control with enhanced security (expect CommandTermination-)
          *********************************************************************/
 
         control = ControlObjectClient_create("simpleIOGenericIO/GGIO1.SPCSO9", con);
 
-        ControlObjectClient_setCommandTerminationHandler(control, commandTerminationHandler, NULL);
-
-        ctlVal = MmsValue_newBoolean(true);
+        if (control)
+        {
+            ControlObjectClient_setCommandTerminationHandler(control, commandTerminationHandler, NULL);
 
-        if (ControlObjectClient_operate(control, ctlVal, 0 /* operate now */)) {
-            printf("simpleIOGenericIO/GGIO1.SPCSO9 operated successfully\n");
-        }
-        else {
-            printf("failed to operate simpleIOGenericIO/GGIO1.SPCSO9\n");
-        }
+            ctlVal = MmsValue_newBoolean(true);
 
-        MmsValue_delete(ctlVal);
+            if (ControlObjectClient_operate(control, ctlVal, 0 /* operate now */)) {
+                printf("simpleIOGenericIO/GGIO1.SPCSO9 operated successfully\n");
+            }
+            else {
+                printf("failed to operate simpleIOGenericIO/GGIO1.SPCSO9\n");
+            }
 
-        /* Wait for command termination message */
-        Thread_sleep(1000);
+            MmsValue_delete(ctlVal);
 
-        ControlObjectClient_destroy(control);
+            /* Wait for command termination message */
+            Thread_sleep(1000);
 
+            ControlObjectClient_destroy(control);
+        }
+        else {
+            printf("Control object simpleIOGenericIO/GGIO1.SPCSO9 not found in server\n");
+        }
 
         IedConnection_close(con);
     }
     else {
     	printf("Connection failed!\n");

Vulnerable code:     if (error == IED_ERROR_OK) {

        MmsValue* ctlVal = MmsValue_newBoolean(true);
        ControlObjectClient_setOrigin(control, NULL, 3);
        if (ControlObjectClient_operate(control, ctlVal, 0 /* operate now */)) {
            printf("simpleIOGenericIO/GGIO1.SPCSO1 operated successfully\n");
        }
        else {
            printf("failed to operate simpleIOGenericIO/GGIO1.SPCSO1\n");
        }
        MmsValue_delete(ctlVal);
        ControlObjectClient_destroy(control);
        /* Check if status value has changed */
        MmsValue* stVal = IedConnection_readObject(con, &error, "simpleIOGenericIO/GGIO1.SPCSO1.stVal", IEC61850_FC_ST);
        if (error == IED_ERROR_OK) {
            bool state = MmsValue_getBoolean(stVal);
            MmsValue_delete(stVal);
            printf("New status of simpleIOGenericIO/GGIO1.SPCSO1.stVal: %i\n", state);
            printf("Reading status for simpleIOGenericIO/GGIO1.SPCSO1 failed!\n");

        if (ControlObjectClient_select(control)) {
            ctlVal = MmsValue_newBoolean(true);
            if (ControlObjectClient_operate(control, ctlVal, 0 /* operate now */)) {
                printf("simpleIOGenericIO/GGIO1.SPCSO2 operated successfully\n");
                printf("failed to operate simpleIOGenericIO/GGIO1.SPCSO2!\n");
            MmsValue_delete(ctlVal);
            printf("failed to select simpleIOGenericIO/GGIO1.SPCSO2!\n");
        ControlObjectClient_destroy(control);


        ControlObjectClient_setCommandTerminationHandler(control, commandTerminationHandler, NULL);
        ctlVal = MmsValue_newBoolean(true);
        if (ControlObjectClient_operate(control, ctlVal, 0 /* operate now */)) {
            printf("simpleIOGenericIO/GGIO1.SPCSO3 operated successfully\n");
        }
        else {
            printf("failed to operate simpleIOGenericIO/GGIO1.SPCSO3\n");
        }
        MmsValue_delete(ctlVal);
        /* Wait for command termination message */
        Thread_sleep(1000);
        ControlObjectClient_destroy(control);
        /* Check if status value has changed */
       stVal = IedConnection_readObject(con, &error, "simpleIOGenericIO/GGIO1.SPCSO3.stVal", IEC61850_FC_ST);
        if (error == IED_ERROR_OK) {
            bool state = MmsValue_getBoolean(stVal);
            printf("New status of simpleIOGenericIO/GGIO1.SPCSO3.stVal: %i\n", state);
            MmsValue_delete(stVal);
            printf("Reading status for simpleIOGenericIO/GGIO1.SPCSO3 failed!\n");
        ControlObjectClient_setCommandTerminationHandler(control, commandTerminationHandler, NULL);
        ctlVal = MmsValue_newBoolean(true);
        if (ControlObjectClient_selectWithValue(control, ctlVal)) {
            if (ControlObjectClient_operate(control, ctlVal, 0 /* operate now */)) {
                printf("simpleIOGenericIO/GGIO1.SPCSO4 operated successfully\n");
                printf("failed to operate simpleIOGenericIO/GGIO1.SPCSO4!\n");
            printf("failed to select simpleIOGenericIO/GGIO1.SPCSO4!\n");
        MmsValue_delete(ctlVal);

        /* Wait for command termination message */
        Thread_sleep(1000);

        ControlObjectClient_destroy(control);


        ControlObjectClient_setCommandTerminationHandler(control, commandTerminationHandler, NULL);

        ctlVal = MmsValue_newBoolean(true);
        if (ControlObjectClient_operate(control, ctlVal, 0 /* operate now */)) {
            printf("simpleIOGenericIO/GGIO1.SPCSO9 operated successfully\n");
        }
        else {
            printf("failed to operate simpleIOGenericIO/GGIO1.SPCSO9\n");
        }
        MmsValue_delete(ctlVal);
        /* Wait for command termination message */
        Thread_sleep(1000);
        ControlObjectClient_destroy(control);
Fixed code:     if (error == IED_ERROR_OK)
    {
        MmsValue* ctlVal = NULL;
        MmsValue* stVal = NULL;
        if (control)
        {
            ctlVal = MmsValue_newBoolean(true);
            ControlObjectClient_setOrigin(control, NULL, 3);
            if (ControlObjectClient_operate(control, ctlVal, 0 /* operate now */)) {
                printf("simpleIOGenericIO/GGIO1.SPCSO1 operated successfully\n");
            }
            else {
                printf("failed to operate simpleIOGenericIO/GGIO1.SPCSO1\n");
            }

            MmsValue_delete(ctlVal);
            ControlObjectClient_destroy(control);
            /* Check if status value has changed */
            stVal = IedConnection_readObject(con, &error, "simpleIOGenericIO/GGIO1.SPCSO1.stVal", IEC61850_FC_ST);
            if (error == IED_ERROR_OK) {
                bool state = MmsValue_getBoolean(stVal);
                MmsValue_delete(stVal);
                printf("New status of simpleIOGenericIO/GGIO1.SPCSO1.stVal: %i\n", state);
            }
            else {
                printf("Reading status for simpleIOGenericIO/GGIO1.SPCSO1 failed!\n");
            }
            printf("Control object simpleIOGenericIO/GGIO1.SPCSO1 not found in server\n");
        if (control)
        {
            if (ControlObjectClient_select(control)) {
                ctlVal = MmsValue_newBoolean(true);
                if (ControlObjectClient_operate(control, ctlVal, 0 /* operate now */)) {
                    printf("simpleIOGenericIO/GGIO1.SPCSO2 operated successfully\n");
                }
                else {
                    printf("failed to operate simpleIOGenericIO/GGIO1.SPCSO2!\n");
                }

                MmsValue_delete(ctlVal);
                printf("failed to select simpleIOGenericIO/GGIO1.SPCSO2!\n");
            ControlObjectClient_destroy(control);
            printf("Control object simpleIOGenericIO/GGIO1.SPCSO2 not found in server\n");
        if (control)
        {
            ControlObjectClient_setCommandTerminationHandler(control, commandTerminationHandler, NULL);
            ctlVal = MmsValue_newBoolean(true);
            if (ControlObjectClient_operate(control, ctlVal, 0 /* operate now */)) {
                printf("simpleIOGenericIO/GGIO1.SPCSO3 operated successfully\n");
            }
            else {
                printf("failed to operate simpleIOGenericIO/GGIO1.SPCSO3\n");
            }
            MmsValue_delete(ctlVal);
            /* Wait for command termination message */
            Thread_sleep(1000);
            ControlObjectClient_destroy(control);
            /* Check if status value has changed */
            stVal = IedConnection_readObject(con, &error, "simpleIOGenericIO/GGIO1.SPCSO3.stVal", IEC61850_FC_ST);
            if (error == IED_ERROR_OK) {
                bool state = MmsValue_getBoolean(stVal);
                printf("New status of simpleIOGenericIO/GGIO1.SPCSO3.stVal: %i\n", state);
                MmsValue_delete(stVal);
            }
            else {
                printf("Reading status for simpleIOGenericIO/GGIO1.SPCSO3 failed!\n");
            }
            printf("Control object simpleIOGenericIO/GGIO1.SPCSO3 not found in server\n");
        if (control)
        {
            ControlObjectClient_setCommandTerminationHandler(control, commandTerminationHandler, NULL);
            ctlVal = MmsValue_newBoolean(true);
            if (ControlObjectClient_selectWithValue(control, ctlVal)) {

                if (ControlObjectClient_operate(control, ctlVal, 0 /* operate now */)) {
                    printf("simpleIOGenericIO/GGIO1.SPCSO4 operated successfully\n");
                }
                else {
                    printf("failed to operate simpleIOGenericIO/GGIO1.SPCSO4!\n");
                }
                printf("failed to select simpleIOGenericIO/GGIO1.SPCSO4!\n");
            MmsValue_delete(ctlVal);

            /* Wait for command termination message */
            Thread_sleep(1000);

            ControlObjectClient_destroy(control);
            printf("Control object simpleIOGenericIO/GGIO1.SPCSO4 not found in server\n");
        if (control)
        {
            ControlObjectClient_setCommandTerminationHandler(control, commandTerminationHandler, NULL);
            ctlVal = MmsValue_newBoolean(true);
            if (ControlObjectClient_operate(control, ctlVal, 0 /* operate now */)) {
                printf("simpleIOGenericIO/GGIO1.SPCSO9 operated successfully\n");
            }
            else {
                printf("failed to operate simpleIOGenericIO/GGIO1.SPCSO9\n");
            }
            MmsValue_delete(ctlVal);
            /* Wait for command termination message */
            Thread_sleep(1000);
            ControlObjectClient_destroy(control);
        }
        else {
            printf("Control object simpleIOGenericIO/GGIO1.SPCSO9 not found in server\n");
        }