Skip to content

Commit

Permalink
Re add len check before copying SNI (#31)
Browse files Browse the repository at this point in the history
Task/Issue URL: https://app.asana.com/0/1202552961248957/1205811169823599/f

### Description
see asana

### Steps to test this PR
- [ ] from this branch, publish the library to maven local ie. `./gradlew clean assemble publishToMavenLocal`
- [ ] In the DDG android app apply the following path
```diff
Subject: [PATCH] Maven local use
---
Index: build.gradle
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/build.gradle b/build.gradle
--- a/build.gradle	(revision d11f7491d7ab4b27223fd352f83c26be403e79ed)
+++ b/build.gradle	(revision 3b1fe446b5d33e4d8a7f400137134ea0b5a797d7)
@@ -40,6 +40,7 @@
     repositories {
         google()
         mavenCentral()
+        mavenLocal()
     }
     configurations.all {
         resolutionStrategy.force 'org.objenesis:objenesis:2.6'
Index: versions.properties
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>ISO-8859-1
===================================================================
diff --git a/versions.properties b/versions.properties
--- a/versions.properties	(revision d11f7491d7ab4b27223fd352f83c26be403e79ed)
+++ b/versions.properties	(revision 3b1fe446b5d33e4d8a7f400137134ea0b5a797d7)
@@ -55,7 +55,7 @@
 
 version.com.android.installreferrer..installreferrer=2.2
 
-version.com.duckduckgo.netguard..netguard-android=1.6.0
+version.com.duckduckgo.netguard..netguard-android=1.7.0-SNAPSHOT
 
 version.com.duckduckgo.synccrypto..sync-crypto-android=0.3.0
 
```
- [ ] build DDG app
- [ ] AppTP smoke tests
  • Loading branch information
aitorvs authored Oct 27, 2023
1 parent e0aa443 commit 9112c3b
Show file tree
Hide file tree
Showing 3 changed files with 123 additions and 4 deletions.
38 changes: 38 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
name: CI Workflow

on:
pull_request:
branches:
- main

jobs:
tls_tests:
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-tls-results
path: test-tls-results.zip
5 changes: 5 additions & 0 deletions src/netguard/tls_parser.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
84 changes: 80 additions & 4 deletions src/test/test_tls.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -469,17 +537,17 @@ 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);
*sn = 0;
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);
Expand All @@ -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;
}

0 comments on commit 9112c3b

Please sign in to comment.