From c8ff35e0f574cfd86fe2c2e000b62bae2924cb1a Mon Sep 17 00:00:00 2001
From: Aitor Viana <aitorvs@gmail.com>
Date: Thu, 26 Oct 2023 18:48:40 +0100
Subject: [PATCH] Re add len check before copying SNI

---
 .github/workflows/ci.yml  | 38 ++++++++++++++++++
 src/netguard/tls_parser.c |  5 +++
 src/test/test_tls.c       | 84 +++++++++++++++++++++++++++++++++++++--
 3 files changed, 123 insertions(+), 4 deletions(-)
 create mode 100644 .github/workflows/ci.yml

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
new file mode 100644
index 0000000..1f59d55
--- /dev/null
+++ b/.github/workflows/ci.yml
@@ -0,0 +1,38 @@
+name: CI Workflow
+
+on:
+  pull_request:
+    branches:
+      - main
+
+jobs:
+  test:
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: Checkout Repository
+        uses: actions/checkout@v2
+
+      - name: Build Test Executable
+        working-directory: ./src/test
+        run: pwd;make clean all
+
+      - name: Run Tests
+        working-directory: ./src/test
+        run: ./test_tls
+
+      - name: Report Test Results
+        run: |
+          if [ $? -eq 0 ]; then
+            echo "All tests passed!"
+          else
+            echo "Tests failed!"
+            exit 1
+          fi
+
+      - name: Upload Test Results
+        if: always()
+        uses: actions/upload-artifact@v3
+        with:
+          name: test-results
+          path: test-results.zip
diff --git a/src/netguard/tls_parser.c b/src/netguard/tls_parser.c
index 07e8c38..92100cc 100644
--- a/src/netguard/tls_parser.c
+++ b/src/netguard/tls_parser.c
@@ -173,6 +173,11 @@ static int parse_server_name_extension(const uint8_t *data, size_t data_len, cha
 
         switch (data[pos]) { /* name type */
             case 0x00: /* host_name */
+                if (len > FQDN_LENGTH) {
+                    log_print(PLATFORM_LOG_PRIORITY_WARN, "TLS SNI too long %d", len);
+                    *hostname = 0;
+                    return -33;
+                }
                 strncpy(hostname, (const char *)(data + pos + 3), len);
                 (hostname)[len] = '\0';
                 return len;
diff --git a/src/test/test_tls.c b/src/test/test_tls.c
index de55781..4351ccb 100644
--- a/src/test/test_tls.c
+++ b/src/test/test_tls.c
@@ -411,6 +411,74 @@ const unsigned char bad_data_4[] = {
         0x01 // Mode: Peer allows to send requests
 };
 
+const unsigned char wrong_sni_length[] = {
+        // TLS record
+        0x16, // Content Type: Handshake
+        0x03, 0x01, // Version: TLS 1.0
+        0x00, 0xec, // Length 104
+        // Handshake
+        0x01, // Handshake Type: Client Hello
+        0x00, 0x00, 0xe8,  // Length 100
+        0x03, 0x01, // Version: TLS 1.0
+        // Random
+        0x4e, 0x55, 0xde, 0x32, 0x80, 0x07, 0x92, 0x9f,
+        0x50, 0x41, 0xe4, 0xf9, 0x58, 0x32, 0xfc, 0x4f,
+        0x10, 0xb3, 0xde, 0x44, 0x4d, 0xa9, 0x67, 0x78,
+        0xea, 0xd1, 0x5f, 0x29, 0x09, 0x04, 0xc1, 0x06,
+        0x00, // Session ID Length
+        0x00, 0x28, // Cipher Suites Length
+        0x00, 0x39,
+        0x00, 0x38,
+        0x00, 0x35,
+        0x00, 0x16,
+        0x00, 0x13,
+        0x00, 0x0a,
+        0x00, 0x33,
+        0x00, 0x32,
+        0x00, 0x2f,
+        0x00, 0x05,
+        0x00, 0x04,
+        0x00, 0x15,
+        0x00, 0x12,
+        0x00, 0x09,
+        0x00, 0x14,
+        0x00, 0x11,
+        0x00, 0x08,
+        0x00, 0x06,
+        0x00, 0x03,
+        0x00, 0xff,
+        0x02, // Compression Methods
+        0x01,
+        0x00,
+        0x00, 0x96, // Extensions Length 18 + 4 + 132 = 150
+        0x00, 0x15, // Extension Type: Padding
+        0x00, 0x80, // Length
+        0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef,
+        0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef,
+        0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef,
+        0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef,
+        0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef,
+        0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef,
+        0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef,
+        0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef,
+        0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef,
+        0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef,
+        0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef,
+        0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef,
+        0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef,
+        0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef,
+        0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef,
+        0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef,
+        0x00, 0x00, // Extension Type: Server Name
+        0x00, 0x0e, // Length
+        0x00, 0x0c, // Server Name Indication Length
+        0x00, // Server Name Type: host_name
+        0xFF, 0xFF, // WRONG Length
+        // "localhost"
+        0x6c, 0x6f, 0x63, 0x61, 0x6c, 0x68, 0x6f, 0x73, 0x74
+};
+
+
 int main() {
     uint8_t *pkt = (uint8_t *)good_data_1;
     int error = 0;
@@ -469,9 +537,9 @@ int main() {
     memset(sn, 0, FQDN_LENGTH);
     *sn = 0;
     error = get_server_name(pkt, sizeof(bad_data_1), pkt, sn);
-    assert(strcmp("localhost", sn) != 0);
-    assert(strlen(sn) == 0);
-    assert(error == -12);
+    assert(strcmp("lodalhost", sn) == 0);
+    assert(strlen(sn) == 9);
+    assert(error == strlen(sn));
 
     pkt = (uint8_t *)bad_data_2;
     memset(sn, 0, FQDN_LENGTH);
@@ -479,7 +547,7 @@ int main() {
     error = get_server_name(pkt, sizeof(bad_data_2), pkt, sn);
     assert(strcmp("localhost", sn) != 0);
     assert(strlen(sn) == 0);
-    assert(error == -12);
+    assert(error == -31);
 
     pkt = (uint8_t *)bad_data_3;
     memset(sn, 0, FQDN_LENGTH);
@@ -489,5 +557,13 @@ int main() {
     assert(strlen(sn) == 0);
     assert(error == -1);
 
+    pkt = (uint8_t *)wrong_sni_length;
+    memset(sn, 0, FQDN_LENGTH);
+    *sn = 0;
+    error = get_server_name(pkt, sizeof(wrong_sni_length), pkt, sn);
+    assert(strcmp("localhost", sn) != 0);
+    assert(strlen(sn) == 0);
+    assert(error == -33);
+
     return 0;
 }