Skip to content

Commit

Permalink
Re add len check before copying SNI
Browse files Browse the repository at this point in the history
  • Loading branch information
aitorvs committed Oct 26, 2023
1 parent e0aa443 commit 578ef0d
Show file tree
Hide file tree
Showing 3 changed files with 125 additions and 4 deletions.
40 changes: 40 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
name: CI Workflow

on:
pull_request:
branches:
- main

jobs:
test:
runs-on: ubuntu-latest

steps:
- name: Checkout Repository
uses: actions/checkout@v2

# - name: Change Directory
# working-directory: ./src/test

- name: Build Test Executable
working-directory: ./src/test
run: pwd;make clean all

- name: Run Tests
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
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 578ef0d

Please sign in to comment.