Project

General

Profile

Actions

Bug #2425

closed

DNP3 memcpy buffer overflow

Added by Philippe Antoine almost 7 years ago. Updated over 6 years ago.

Status:
Closed
Priority:
High
Assignee:
Target version:
Affected Versions:
Effort:
Difficulty:
Label:

Description

In the file in src/app-layer-dnp3-objects.c, there are buffer overflows due to memcpy executions after checks on the written-to buffer, but not the read from buffer.

For instance in function DNP3DecodeObjectG70V3, we have the following code

        if (!DNP3ReadUint16(buf, len, &object->filename_size)) {
            goto error;
        }
       //other DNP3ReadUintXX...
        if (object->filename_size > 0) {
            memcpy(object->filename, *buf, object->filename_size);
            *buf += object->filename_size;
            *len -= object->filename_size;
        }

The check is missing on buffer *buf length (ie *len) versus object->filename_size (which comes from the packet).

The error comes form the python script generating the code.
Patch should be like

diff --git a/scripts/dnp3-gen/dnp3-gen.py b/scripts/dnp3-gen/dnp3-gen.py
index cc2aa8fc..f0bf1c0a 100755
--- a/scripts/dnp3-gen/dnp3-gen.py
+++ b/scripts/dnp3-gen/dnp3-gen.py
@@ -186,6 +186,10 @@ void OutputJsonDNP3SetItem(json_t *js, DNP3Object *object,
             json_object_set_new(js, "data->{{field.name}}", json_string(data->{{field.name}}));
 {% elif field.type == "chararray" %}
             if (data->{{field.len_field}} > 0) {
+                if (*len < data->{{field.len_field}}) {
+                    /* Not enough data. */
+                    goto error;
+                }
                 /* First create a null terminated string as not all versions
                  * of jansson have json_stringn. */
                 char tmpbuf[data->{{field.len_field}} + 1];
@@ -527,6 +531,10 @@ static int DNP3DecodeObjectG{{object.group}}V{{object.variation}}(const uint8_t
         object->{{field.len_field}} = prefix - (offset - *len);
 {% endif %}
         if (object->{{field.len_field}} > 0) {
+            if (*len < object->{{field.len_field}}) {
+                /* Not enough data. */
+                goto error;
+            }
             memcpy(object->{{field.name}}, *buf, object->{{field.len_field}});
             *buf += object->{{field.len_field}};
             *len -= object->{{field.len_field}};


Files

off.pcap (146 Bytes) off.pcap Pcap triggering the overflow with config file setting midstream:true Philippe Antoine, 01/25/2018 02:20 AM
p2425.diff (8.02 KB) p2425.diff Proposed patch Philippe Antoine, 01/25/2018 05:04 AM
0001-Fixes-Bug-2425.patch (7.34 KB) 0001-Fixes-Bug-2425.patch patch with git format-patch Philippe Antoine, 01/25/2018 10:03 AM
issue-2425-4.0.x.tar (20 KB) issue-2425-4.0.x.tar 4.0.x patch set. Jason Ish, 01/26/2018 04:23 PM
issue-2425-master.tar (20 KB) issue-2425-master.tar Master patch set. Jason Ish, 01/26/2018 04:23 PM
Actions

Also available in: Atom PDF