<p>Pau Espin Pedrol has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/11280">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">server: Improve verification of messages from client<br><br>Take the chance to define SERVER_MAX_DATA_SIZE as pcap payload, which we<br>can later match to configurable snaplen parameter.<br><br>Change-Id: I45d4c59026faf1108c0976eb6ad8c270e3577dbf<br>---<br>M include/osmo-pcap/osmo_pcap_server.h<br>M src/osmo_server_network.c<br>2 files changed, 38 insertions(+), 8 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/osmo-pcap refs/changes/80/11280/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/include/osmo-pcap/osmo_pcap_server.h b/include/osmo-pcap/osmo_pcap_server.h</span><br><span>index c1d318e..cdcdb70 100644</span><br><span>--- a/include/osmo-pcap/osmo_pcap_server.h</span><br><span>+++ b/include/osmo-pcap/osmo_pcap_server.h</span><br><span>@@ -91,7 +91,7 @@</span><br><span>   int state;</span><br><span>   int pend;</span><br><span>    int reopen;</span><br><span style="color: hsl(0, 100%, 40%);">-     char buf[SERVER_MAX_DATA_SIZE + sizeof(struct osmo_pcap_data)];</span><br><span style="color: hsl(120, 100%, 40%);">+       char buf[sizeof(struct osmo_pcap_data) + sizeof(struct osmo_pcap_pkthdr) + SERVER_MAX_DATA_SIZE];</span><br><span>    struct osmo_pcap_data *data;</span><br><span> </span><br><span>     /* statistics */</span><br><span>diff --git a/src/osmo_server_network.c b/src/osmo_server_network.c</span><br><span>index 695090d..8eb7567 100644</span><br><span>--- a/src/osmo_server_network.c</span><br><span>+++ b/src/osmo_server_network.c</span><br><span>@@ -192,12 +192,15 @@</span><br><span> {</span><br><span>       struct pcap_file_header *hdr;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-       if (data->len != sizeof(*hdr)) {</span><br><span style="color: hsl(0, 100%, 40%);">-             LOGP(DSERVER, LOGL_ERROR, "The pcap_file_header does not fit.\n");</span><br><span style="color: hsl(120, 100%, 40%);">+  hdr = (struct pcap_file_header *) &data->data[0];</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+    if (hdr->snaplen > SERVER_MAX_DATA_SIZE) {</span><br><span style="color: hsl(120, 100%, 40%);">+              LOGP(DSERVER, LOGL_ERROR,</span><br><span style="color: hsl(120, 100%, 40%);">+                  "The recvd pcap_file_header contains too big snaplen %zu > %zu\n",</span><br><span style="color: hsl(120, 100%, 40%);">+               (size_t) hdr->snaplen, (size_t) SERVER_MAX_DATA_SIZE);</span><br><span>               return -1;</span><br><span>   }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-   hdr = (struct pcap_file_header *) &data->data[0];</span><br><span>     if (!conn->no_store && conn->local_fd < 0) {</span><br><span>                conn->file_hdr = *hdr;</span><br><span>            restart_pcap(conn);</span><br><span>@@ -335,6 +338,36 @@</span><br><span>   return do_read_tls(conn, buf, size);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+static bool pcap_data_valid(struct osmo_pcap_conn *conn)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+    unsigned int min_len, max_len;</span><br><span style="color: hsl(120, 100%, 40%);">+        switch ((enum OsmoPcapDataType) conn->data->type) {</span><br><span style="color: hsl(120, 100%, 40%);">+     case PKT_LINK_HDR:</span><br><span style="color: hsl(120, 100%, 40%);">+            if (conn->data->len != sizeof(struct pcap_file_header)) {</span><br><span style="color: hsl(120, 100%, 40%);">+                       LOGP(DSERVER, LOGL_ERROR,</span><br><span style="color: hsl(120, 100%, 40%);">+                          "Implausible llink_hdr length: %u != %zu\n",</span><br><span style="color: hsl(120, 100%, 40%);">+                        conn->data->len, sizeof(struct osmo_pcap_pkthdr));</span><br><span style="color: hsl(120, 100%, 40%);">+                 return false;</span><br><span style="color: hsl(120, 100%, 40%);">+         }</span><br><span style="color: hsl(120, 100%, 40%);">+             break;</span><br><span style="color: hsl(120, 100%, 40%);">+        case PKT_LINK_DATA:</span><br><span style="color: hsl(120, 100%, 40%);">+           min_len = sizeof(struct osmo_pcap_pkthdr);</span><br><span style="color: hsl(120, 100%, 40%);">+            max_len = SERVER_MAX_DATA_SIZE + sizeof(struct osmo_pcap_pkthdr);</span><br><span style="color: hsl(120, 100%, 40%);">+             if (conn->data->len < min_len || conn->data->len > max_len) {</span><br><span style="color: hsl(120, 100%, 40%);">+                       LOGP(DSERVER, LOGL_ERROR,</span><br><span style="color: hsl(120, 100%, 40%);">+                          "Implausible data length: %u < %u <= %u\n",</span><br><span style="color: hsl(120, 100%, 40%);">+                           min_len, conn->data->len, max_len);</span><br><span style="color: hsl(120, 100%, 40%);">+                        return false;</span><br><span style="color: hsl(120, 100%, 40%);">+         }</span><br><span style="color: hsl(120, 100%, 40%);">+             break;</span><br><span style="color: hsl(120, 100%, 40%);">+        default:</span><br><span style="color: hsl(120, 100%, 40%);">+              LOGP(DSERVER, LOGL_ERROR, "Unknown data type %" PRIx8 "\n",</span><br><span style="color: hsl(120, 100%, 40%);">+                    conn->data->type);</span><br><span style="color: hsl(120, 100%, 40%);">+         return false;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+     return true;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> static int read_cb_initial(struct osmo_pcap_conn *conn)</span><br><span> {</span><br><span>    int rc;</span><br><span>@@ -354,11 +387,8 @@</span><br><span>       } else if (conn->pend == 0) {</span><br><span>             conn->data->len = ntohs(conn->data->len);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-               if (conn->data->len > SERVER_MAX_DATA_SIZE) {</span><br><span style="color: hsl(0, 100%, 40%);">-                  LOGP(DSERVER, LOGL_ERROR,</span><br><span style="color: hsl(0, 100%, 40%);">-                            "Implausible data length: %u\n", conn->data->len);</span><br><span style="color: hsl(120, 100%, 40%);">+               if (!pcap_data_valid(conn))</span><br><span>                  return -1;</span><br><span style="color: hsl(0, 100%, 40%);">-              }</span><br><span> </span><br><span>                conn->state = STATE_DATA;</span><br><span>                 conn->pend = conn->data->len;</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/11280">change 11280</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/11280"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-pcap </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: I45d4c59026faf1108c0976eb6ad8c270e3577dbf </div>
<div style="display:none"> Gerrit-Change-Number: 11280 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Pau Espin Pedrol <pespin@sysmocom.de> </div>