Skip to content

Commit da4cc05

Browse files
committed
Bug Fix: Fix hardening issues as spotted on #115
This commit fixes two hardening issues found by @BrendonIrwan: 1) Wrong casts to (char*) 2) Wrong return code for tws_receiveframe(): since a frame can be up to 64-bit size, the 'int' type is unable to hold the entire result value. The fix is simple: this function now returns an 'uint64_t' and accepts an additinal 'int *err' parameter, which will be set to signal if the operation was successful or not.
1 parent 5ef631d commit da4cc05

8 files changed

Lines changed: 65 additions & 28 deletions

File tree

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
# along with this program. If not, see <http://www.gnu.org/licenses/>
1515

1616
# Ignore object files, executables and the library.
17+
*~
1718
*.o
1819
*.a
1920
ws_file

Makefile

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,17 @@ AFL_FUZZ ?= no
2727
VERBOSE_EXAMPLES ?= yes
2828
VALIDATE_UTF8 ?= yes
2929

30+
# Compiler base and security warning options.
31+
CFLAGS += -g -std=c2x -Wall -Wextra -Wpedantic -Warray-bounds=2 \
32+
-Wcast-qual -Wformat-overflow=2 -Wformat-truncation=2 -Wformat=2 \
33+
-Wnull-dereference -Wshift-overflow=2 -Wstack-protector \
34+
-Wstringop-overflow=4 -Wtrampolines -Wvla
35+
# Compiler hardening options.
36+
CFLAGS += -fPIE -fsanitize-undefined-trap-on-error -fsanitize=undefined \
37+
-fstack-clash-protection -fstack-protector-strong
38+
# Preprocessor hardening options.
39+
CFLAGS += -D_FORTIFY_SOURCE=3
40+
3041
# Prefix
3142
ifeq ($(PREFIX),)
3243
PREFIX := /usr/local

examples/echo/echo.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ void onmessage(ws_cli_conn_t client,
105105
* ws_sendframe_bin()
106106
* ws_sendframe_bin_bcast()
107107
*/
108-
ws_sendframe_bcast(8080, (char *)msg, size, type);
108+
ws_sendframe_bcast(8080, (const char*)msg, size, type);
109109
}
110110

111111
/**

extra/toyws/README.md

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,8 @@ Returns 0 if success, otherwise, a negative number.
6363
---
6464
6565
```c
66-
int tws_receiveframe(struct tws_ctx *ctx, char **buff, size_t *buff_size,
67-
int *frm_type);
66+
uint64_t tws_receiveframe(struct tws_ctx *ctx, char **buff, size_t *buff_size,
67+
int *frm_type, int *err);
6868
```
6969
Receive a frame and save it on `buff`.
7070

@@ -89,7 +89,11 @@ points to NULL, `*buff_size` must be equals to 0.
8989
Frame type read. The frame type received will be reflected into the contents of
9090
this pointer.
9191

92-
**Return**: Returns 0 if success, a negative number otherwise.
92+
**`err`:**
93+
94+
Output error code. Set to a negative value on error, 0 on success.
95+
96+
**Return**: Returns frame length on success, 0 on error (check `*err`).
9397

9498
**Note**:
9599

@@ -114,10 +118,12 @@ int main(void)
114118
char msg[] = "Hello";
115119

116120
/* Buffer/frame params. */
121+
int err;
117122
char *buff;
118123
int frm_type;
119124
size_t buff_size;
120125

126+
err = 0;
121127
buff = NULL;
122128
buff_size = 0;
123129
frm_type = 0;
@@ -131,7 +137,8 @@ int main(void)
131137
"Success" : "Failed"));
132138

133139
/* Blocks until receive a single message. */
134-
if (tws_receiveframe(&ctx, &buff, &buff_size, &frm_type) < 0)
140+
tws_receiveframe(&ctx, &buff, &buff_size, &frm_type, &err);
141+
if (err < 0)
135142
fprintf(stderr, "Unable to receive message!\n");
136143

137144
printf("I received: (%s) (type: %s)\n", buff,

extra/toyws/toyws.c

Lines changed: 33 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -317,12 +317,12 @@ static int skip_frame(struct tws_ctx *ctx, uint64_t frame_size)
317317
* @param buff Buffer pointer.
318318
* @param buff_size Buffer size.
319319
* @param frm_type Frame type received.
320+
* @param err Output error code: set to a negative value on error, 0 on success.
320321
*
321-
* @return Returns frame length if success, a negative number
322-
* otherwise.
322+
* @return Returns frame length if success, 0 on error (check @p err).
323323
*/
324-
int tws_receiveframe(struct tws_ctx *ctx, char **buff,
325-
size_t *buff_size, int *frm_type)
324+
uint64_t tws_receiveframe(struct tws_ctx *ctx, char **buff,
325+
size_t *buff_size, int *frm_type, int *err)
326326
{
327327
int ret;
328328
char *buf;
@@ -331,9 +331,14 @@ int tws_receiveframe(struct tws_ctx *ctx, char **buff,
331331
uint8_t opcode;
332332
uint64_t frame_length;
333333

334+
if (err)
335+
*err = 0;
336+
334337
/* Buffer should be valid. */
335-
if (!buff || (*buff && !buff_size))
336-
return (-1);
338+
if (!buff || (*buff && !buff_size)) {
339+
if (err) *err = -1;
340+
return (0);
341+
}
337342

338343
ret = 0;
339344
cur_byte = next_byte(ctx, &ret);
@@ -343,7 +348,8 @@ int tws_receiveframe(struct tws_ctx *ctx, char **buff,
343348
if (opcode == FRM_CLSE)
344349
{
345350
tws_close(ctx);
346-
return (-1);
351+
if (err) *err = -1;
352+
return (0);
347353
}
348354

349355
frame_length = next_byte(ctx, &ret) & 0x7F;
@@ -369,23 +375,29 @@ int tws_receiveframe(struct tws_ctx *ctx, char **buff,
369375
}
370376

371377
/* Check if any error before proceed. */
372-
if (ret)
373-
return (ret);
378+
if (ret) {
379+
if (err) *err = ret;
380+
return (0);
381+
}
374382

375383
/*
376384
* If frame is something other than CLSE and TXT/BIN (like PONG
377385
* or CONT), skip.
378386
*/
379387
if (opcode != FRM_TXT && opcode != FRM_BIN)
380-
if (skip_frame(ctx, frame_length) < 0)
381-
return (-1);
388+
if (skip_frame(ctx, frame_length) < 0) {
389+
if (err) *err = -1;
390+
return (0);
391+
}
382392

383-
/* Allocate memory, if needed. */
384-
if (*buff_size < frame_length)
393+
/* Allocate memory, if needed (+1 for null terminator, covers 0-length frames). */
394+
if (*buff_size < frame_length + 1)
385395
{
386396
buf = realloc(*buff, frame_length + 1);
387-
if (!buf)
388-
return (-1);
397+
if (!buf) {
398+
if (err) *err = -1;
399+
return (0);
400+
}
389401
*buff = buf;
390402
*buff_size = frame_length + 1;
391403
}
@@ -396,8 +408,10 @@ int tws_receiveframe(struct tws_ctx *ctx, char **buff,
396408
for (i = 0; i < frame_length; i++, buf++)
397409
{
398410
cur_byte = next_byte(ctx, &ret);
399-
if (cur_byte < 0)
400-
return (ret == 0 ? frame_length : ret);
411+
if (cur_byte < 0) {
412+
if (err) *err = ret;
413+
return (ret == 0 ? frame_length : 0);
414+
}
401415

402416
*buf = cur_byte;
403417
}
@@ -406,5 +420,6 @@ int tws_receiveframe(struct tws_ctx *ctx, char **buff,
406420
/* Fill other infos. */
407421
*frm_type = opcode;
408422

409-
return (ret == 0 ? frame_length : ret);
423+
if (err) *err = ret;
424+
return (ret == 0 ? frame_length : 0);
410425
}

extra/toyws/toyws.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@ extern "C" {
5353
extern void tws_close(struct tws_ctx *ctx);
5454
extern int tws_sendframe(struct tws_ctx *ctx, uint8_t *msg,
5555
uint64_t size, int type);
56-
extern int tws_receiveframe(struct tws_ctx *ctx, char **buff,
57-
size_t *buff_size, int *frm_type);
56+
extern uint64_t tws_receiveframe(struct tws_ctx *ctx, char **buff,
57+
size_t *buff_size, int *frm_type, int *err);
5858

5959
#ifdef __cplusplus
6060
}

extra/toyws/tws_test.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,12 @@ int main(void)
2626
char msg[] = "Hello";
2727

2828
/* Buffer params. */
29+
int err;
2930
char *buff;
3031
int frm_type;
3132
size_t buff_size;
3233

34+
err = 0;
3335
buff = NULL;
3436
buff_size = 0;
3537
frm_type = 0;
@@ -43,7 +45,8 @@ int main(void)
4345
"Success" : "Failed"));
4446

4547
/* Blocks until receive a single message. */
46-
if (tws_receiveframe(&ctx, &buff, &buff_size, &frm_type) < 0)
48+
tws_receiveframe(&ctx, &buff, &buff_size, &frm_type, &err);
49+
if (err < 0)
4750
fprintf(stderr, "Unable to receive message!\n");
4851

4952
printf("I received: (%s) (type: %s)\n", buff,

src/handshake.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,12 +82,12 @@ int get_handshake_accept(char *wsKey, unsigned char **dest)
8282
* @returns If found, returns a pointer at the beginning of the
8383
* found substring. Otherwise, returns NULL.
8484
*/
85-
static char *strstricase(const char *haystack, const char *needle)
85+
static const char *strstricase(const char *haystack, const char *needle)
8686
{
8787
size_t length;
8888
for (length = strlen(needle); *haystack; haystack++)
8989
if (!strncasecmp(haystack, needle, length))
90-
return (char*)haystack;
90+
return haystack;
9191
return (NULL);
9292
}
9393

0 commit comments

Comments
 (0)