Changeset 11124 in ntrip

Timestamp:

Oct 24, 2026, 10:10:29 PM (6 hours ago)

Author:

stuerze

Message:

Some fixes regarding memory corruption from stream data, credential exposure,

Location:

trunk/BNC

Files:

• CHANGELOG.md (modified) (1 diff)
• src/RTCM3/RTCM3Decoder.cpp (modified) (1 diff)
• src/RTCM3/clock_and_orbit/clock_orbit.h (modified) (1 diff)
• src/RTCM3/clock_and_orbit/clock_orbit_rtcm_new.cpp (modified) (2 diffs)
• src/RTCM3/crs.h (modified) (5 diffs)
• src/bncnetqueryrtp.cpp (modified) (6 diffs)
• src/bncnetqueryv1.cpp (modified) (1 diff)
• src/upload/bncrtnetuploadcaster.cpp (modified) (1 diff)
• tests/README.md (modified) (2 diffs)

trunk/BNC/CHANGELOG.md

@@ -52 +52 @@
 - FIXED: RINEX QC used the first ephemeris of the file for all epochs (wrong GLONASS elevations, very long run times for daily files)
 - FIXED: Ntrip Version 2 streams: longer timeout for the first read after (re)connect, allowing for slow proxy auto-detection and TLS handshake
+- FIXED: Memory corruption by stream content: VTEC messages with spherical harmonics degree or order 16 (IGS-SSR and RTCM-SSR; with 4 layers BNC crashed), RTCM-SSR metadata with more than 7 entries, names of 31 characters in RTCM 1300/1301/1302; range check of the VTEC parameters from the RTNet engine
+- FIXED: Password no longer sent in the request line (Ntrip Version 1 via proxy, RTSP) and no longer written to the log (RTSP)
 
 ## 2.13.7 (2026-07-31)

trunk/BNC/src/RTCM3/RTCM3Decoder.cpp

@@ -2173 +2173 @@
     for (int i = 0; i < dblinksnum; i++) {
       int dblinknum = -1;
-      char dblinkname[31];
+      char dblinkname[32]; // up to 31 characters (5-bit length) + terminating 0
       GETBITS(dblinknum, 5)
       if (dblinknum > -1 && dblinknum <= 31) {

trunk/BNC/src/RTCM3/clock_and_orbit/clock_orbit.h

@@ -454 +454 @@
       unsigned int Degree; /* 1-16 */
       unsigned int Order;  /* 1-16 */
-      double Sinus[VTEC_MAXIONODEGREE][VTEC_MAXIONOORDER];
-      double Cosinus[VTEC_MAXIONODEGREE][VTEC_MAXIONOORDER];
+      /* indices 0 .. Degree / 0 .. Order, both up to 16 */
+      double Sinus[VTEC_MAXIONODEGREE+1][VTEC_MAXIONOORDER+1];
+      double Cosinus[VTEC_MAXIONODEGREE+1][VTEC_MAXIONOORDER+1];
     } Layers[VTEC_NUMIONOLAYERS];
   };

trunk/BNC/src/RTCM3/clock_and_orbit/clock_orbit_rtcm_new.cpp

@@ -637 +637 @@
       fprintf(stderr, "ssrIod %d providerId %d solId %d numEntries %d \n",
       md->SSRIOD, md->SSRProviderID, md->SSRSolutionID, md->NumEntries);
-#endif 
+#endif
+    if (md->NumEntries > META_MAXNUMENTRIES) { /* DF+063 is 5 bits wide */
+      md->NumEntries = 0;
+      return GCOBR_DATAMISMATCH;
+    }
     for (i = 0; i < md->NumEntries; i++) {
       unsigned int additionalBits;
@@ -687 +691 @@
       if (md->Entries[i].nonDefaultIndicator &&
         md->Entries[i].additionalParameterIndicator) {
-        D_ADDITIONAL_MODEL_PARAMETER_DATA(md->Entries[i].additionalParameterData, additionalBits)
+        /* DF+071 allows up to 255 bits, only 32 can be stored: skip the
+           leading excess bits, keep the trailing 32 */
+        while (additionalBits > 32) {
+          unsigned int n = (additionalBits - 32 > 32) ? 32 : additionalBits - 32;
+          SKIPSSRBITS(n)
+          additionalBits -= n;
+        }
+        if (additionalBits)
+          D_ADDITIONAL_MODEL_PARAMETER_DATA(md->Entries[i].additionalParameterData, additionalBits)
 #ifdef BNC_DEBUG_SSR 
         fprintf(stderr, "- additional-data-parameter-data     : %d\n", md->Entries[i].additionalParameterData);

trunk/BNC/src/RTCM3/crs.h

@@ -24 +24 @@
   }
   void clear(void) {
-    memset(_name, '\0', 31 * sizeof(char));
+    memset(_name, '\0', sizeof(_name));
     _coordinateEpoch = 0;
     _CE              = 0;
@@ -38 +38 @@
     void setCoordinateEpochFromCE() {(!_CE) ? _coordinateEpoch = 0.0 : _coordinateEpoch = 1900 + _CE; }
     void setCEFromCoordinateEpoch() {(!_coordinateEpoch) ? _CE = 0 : _CE = _coordinateEpoch -1900; }
-    char     _name[31];
+    char     _name[32]; // up to 31 characters (5-bit length) + terminating 0
     double   _coordinateEpoch;
     double   _CE;
@@ -55 +55 @@
     }
     void clear() {
-      memset(_name, '\0', 31 * sizeof(char));
+      memset(_name, '\0', sizeof(_name));
       _anchor = 0;
       _plateNumber = 0;
@@ -71 +71 @@
       }
     }
-    char             _name[31];
+    char             _name[32]; // up to 31 characters (5-bit length) + terminating 0
     int              _anchor;           // 0.. global CRS (=> Plate number 0), 1.. plate-fixed CRS
     int              _plateNumber;      // 0.. unknown, 7.. Eurasia
@@ -171 +171 @@
        cout << endl;
      }
-     char           _sourceName[31];
-     char           _targetName[31];
+     char           _sourceName[32]; // up to 31 characters (5-bit length) + terminating 0
+     char           _targetName[32];
      int            _sysIdentNum;
      int            _utilTrafoMessageIndicator;

trunk/BNC/src/bncnetqueryrtp.cpp

@@ -47 +47 @@
   _status = finished;
   if (_socket) {
-    QByteArray reqStr = "TEARDOWN " + _url.toEncoded() + " RTSP/1.0\r\n"
+    QByteArray reqStr = "TEARDOWN " + _url.toEncoded(QUrl::RemoveUserInfo) + " RTSP/1.0\r\n"
                       + "CSeq: " + QString("%1").arg(++_CSeq).toLatin1() + "\r\n"
                       + "Session: " + _session + "\r\n"
@@ -59 +59 @@
 void bncNetQueryRtp::slotKeepAlive() {
   if (_socket) {
-    QByteArray reqStr = "GET_PARAMETER " + _url.toEncoded() + " RTSP/1.0\r\n"
+    QByteArray reqStr = "GET_PARAMETER " + _url.toEncoded(QUrl::RemoveUserInfo) + " RTSP/1.0\r\n"
                       + "CSeq: " + QString("%1").arg(++_CSeq).toLatin1() + "\r\n"
                       + "Session: " + _session + "\r\n"
@@ -144 +144 @@
 
     QByteArray reqStr;
-    reqStr = "SETUP " + _url.toEncoded() + " RTSP/1.0\r\n"
+    reqStr = "SETUP " + _url.toEncoded(QUrl::RemoveUserInfo) + " RTSP/1.0\r\n"
            + "CSeq: " + QString("%1").arg(++_CSeq).toLatin1() + "\r\n"
            + "Ntrip-Version: Ntrip/2.0\r\n"
@@ -202 +202 @@
           }
 
-          reqStr = "PLAY " + _url.toEncoded() + " RTSP/1.0\r\n"
+          reqStr = "PLAY " + _url.toEncoded(QUrl::RemoveUserInfo) + " RTSP/1.0\r\n"
                  + "CSeq: " + QString("%1").arg(++_CSeq).toLatin1() + "\r\n"
                  + "Session: " + _session + "\r\n"
@@ -216 +216 @@
               while (!line.isEmpty()) {
                 if (line.indexOf("200 OK") != -1) {
-                  emit newMessage(_url.toEncoded().replace(0,1,"")
+                  emit newMessage(_url.toEncoded(QUrl::RemoveUserInfo).replace(0,1,"")
                             + ": UDP connection established", true);
                   slotKeepAlive();
@@ -233 +233 @@
   _socket = 0;
   _status = error;
-  emit newMessage(_url.toEncoded().replace(0,1,"")
+  emit newMessage(_url.toEncoded(QUrl::RemoveUserInfo).replace(0,1,"")
                   + ": NetQuery, waiting for connect", true);
 }

trunk/BNC/src/bncnetqueryv1.cpp

@@ -225 +225 @@
              + userAndPwd + "\r\n";
   } else {
-    reqStr = "GET " + _url.toEncoded() + " HTTP/1.0\r\n"
+    reqStr = "GET " + _url.toEncoded(QUrl::RemoveUserInfo) + " HTTP/1.0\r\n"
              + "User-Agent: NTRIP BNC/" BNCVERSION " (" BNC_OS ")\r\n"
              + "Host: " + _url.host().toLatin1() + "\r\n"

trunk/BNC/src/upload/bncrtnetuploadcaster.cpp

@@ -504 +504 @@
             in >> ui >> vtec.NumLayers;
             vtec.UpdateInterval = (unsigned int)determineUpdateInd(ui);
+            if (vtec.NumLayers > VTEC_NUMIONOLAYERS) {
+                vtec.NumLayers = 0;
+            }
             for (unsigned ll = 0; ll < vtec.NumLayers; ll++) {
                 int dummy;
                 in >> dummy >> vtec.Layers[ll].Degree >> vtec.Layers[ll].Order
                     >> vtec.Layers[ll].Height;
+                if (vtec.Layers[ll].Degree < 1 || vtec.Layers[ll].Degree > VTEC_MAXIONODEGREE ||
+                    vtec.Layers[ll].Order  < 1 || vtec.Layers[ll].Order  > VTEC_MAXIONOORDER) {
+                    vtec.NumLayers = 0;
+                    break;
+                }
                 for (unsigned iDeg = 0; iDeg <= vtec.Layers[ll].Degree; iDeg++) {
                     for (unsigned iOrd = 0; iOrd <= vtec.Layers[ll].Order; iOrd++) {

trunk/BNC/tests/README.md

@@ -28 +28 @@
 | `tools/fakecaster.py` | local Ntrip caster recording an upload as a BNC raw file (case key `fakecaster`) |
 | `tools/bncraw.py` | `info` (streams, time span, RTCM3 message types) and `slice` for raw captures |
+| `tools/mkcrafted.py` | writes `data/crafted_limits.raw`: crafted frames at the upper field limits |
+| `tools/ssr_harness.sh` | builds `tools/ssr_harness.cpp` with the SSR decoders under ASan/UBSan and runs it |
 | `work/` | scratch output, not versioned |
 
@@ -90 +92 @@
   'QC Bias File' (code bias check against the other ACs and the file).
 
+* `decoder_limits`: crafted, CRC-valid frames from `tools/mkcrafted.py`
+  using the largest values the formats allow: VTEC with degree/order 16
+  (IGS-SSR 4076/201 and RTCM-SSR 1264, 2 and 4 layers) and RTCM 1300/1301/1302
+  with 31-character names, listed by the RTCM scan in the log. The
+  coefficients are C(d,o) = (100*d+o)/200 TECU and S = -C, both `.ssr` files
+  are identical apart from the station ID. These frames overflowed the
+  coefficient and name arrays; the 4-layer VTEC crashed BNC.
+
+## Sanitizer check of the SSR decoders
+
+```
+tests/tools/ssr_harness.sh            # crafted_limits.raw
+tests/tools/ssr_harness.sh tests/data/*.raw   # real captures
+```
+
+In BNC all SSR target structs are members of one `RTCM3coDecoder` object,
+so a write beyond one struct corrupts the next member without any
+AddressSanitizer report. The harness decodes every frame with all three
+format classes into separately allocated structs, so ASan reports it. It
+also covers the new RTCM-SSR metadata (message 60 in
+`crafted_limits.raw`: 7 entries with additional parameters of 40 and 0 bits,
+and 31 entries, which must be rejected), which `RTCM3Decoder.cpp` does not
+dispatch at present. Exit code 0 only without sanitizer errors.
+
 ## Adding a replay case