Skip to content

Commit 8792819

Browse files
wberubewberube
committed
Hardening server security, resolving race conditions and improving
signal delivery
1 parent 088cf32 commit 8792819

3 files changed

Lines changed: 68 additions & 50 deletions

File tree

‎src/jpeg.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
int jpeg_index;
44
bool jpeg_module_init = false;
55

6-
pthread_mutex_t jpeg_mutex;
6+
pthread_mutex_t jpeg_mutex = PTHREAD_MUTEX_INITIALIZER;
77

88
int jpeg_init() {
99
int ret;

‎src/main.c‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,16 +34,23 @@ void handle_exit(int signo) {
3434

3535
int main(int argc, char *argv[]) {
3636
{
37-
char signal_error[] = {SIGABRT, SIGBUS, SIGFPE, SIGILL, SIGSEGV};
38-
char signal_exit[] = {SIGHUP, SIGINT, SIGQUIT, SIGTERM};
39-
char signal_null[] = {EPIPE, SIGPIPE};
40-
41-
for (char *s = signal_error; s < (&signal_error)[1]; s++)
42-
signal(*s, handle_error);
43-
for (char *s = signal_exit; s < (&signal_exit)[1]; s++)
44-
signal(*s, handle_exit);
45-
for (char *s = signal_null; s < (&signal_null)[1]; s++)
46-
signal(*s, NULL);
37+
struct sigaction sa;
38+
memset(&sa, 0, sizeof(sa));
39+
sa.sa_handler = handle_error;
40+
sigaction(SIGABRT, &sa, NULL);
41+
sigaction(SIGBUS, &sa, NULL);
42+
sigaction(SIGFPE, &sa, NULL);
43+
sigaction(SIGILL, &sa, NULL);
44+
sigaction(SIGSEGV, &sa, NULL);
45+
46+
sa.sa_handler = handle_exit;
47+
sigaction(SIGHUP, &sa, NULL);
48+
sigaction(SIGINT, &sa, NULL);
49+
sigaction(SIGQUIT, &sa, NULL);
50+
sigaction(SIGTERM, &sa, NULL);
51+
52+
sa.sa_handler = SIG_IGN;
53+
sigaction(SIGPIPE, &sa, NULL);
4754
}
4855

4956
hal_identify();

‎src/server.c‎

Lines changed: 50 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ void send_h26x_to_client(char index, hal_vidstream *stream) {
159159
pack->nalu[j].type != NalUnitType_SPS_HEVC)
160160
continue;
161161

162-
static char len_buf[16];
162+
char len_buf[16];
163163
int len_size = sprintf(len_buf, "%zX\r\n", pack->nalu[j].length);
164164

165165
struct iovec iov[3];
@@ -207,7 +207,7 @@ void send_mp4_to_client(char index, hal_vidstream *stream, char isH265) {
207207
}
208208

209209
static enum BufError err;
210-
static char len_buf[50];
210+
char len_buf[50];
211211
pthread_mutex_lock(&client_fds_mutex);
212212
for (unsigned int i = 0; i < HTTP_MAX_CLIENTS; ++i) {
213213
if (client_fds[i].sockFd < 0) continue;
@@ -280,7 +280,7 @@ void send_mp3_to_client(char *buf, ssize_t size) {
280280
if (client_fds[i].sockFd < 0) continue;
281281
if (client_fds[i].type != STREAM_MP3) continue;
282282

283-
static char len_buf[50];
283+
char len_buf[50];
284284
ssize_t len_size = sprintf(len_buf, "%zX\r\n", size);
285285
if (send_to_client(i, len_buf, len_size) < 0)
286286
continue; // send <SIZE>\r\n
@@ -298,7 +298,7 @@ void send_pcm_to_client(hal_audframe *frame) {
298298
if (client_fds[i].sockFd < 0) continue;
299299
if (client_fds[i].type != STREAM_PCM) continue;
300300

301-
static char len_buf[50];
301+
char len_buf[50];
302302
ssize_t len_size = sprintf(len_buf, "%zX\r\n", frame->length[0]);
303303
if (send_to_client(i, len_buf, len_size) < 0)
304304
continue; // send <SIZE>\r\n
@@ -366,18 +366,19 @@ struct jpegtask {
366366
};
367367

368368
void *send_jpeg_thread(void *vargp) {
369-
struct jpegtask task = *((struct jpegtask *)vargp);
369+
struct jpegtask *task = (struct jpegtask *)vargp;
370370
hal_jpegdata jpeg = {0};
371371
HAL_INFO("server", "Requesting a JPEG snapshot (%ux%u, qfactor %u, color2Gray %d)...\n",
372-
task.width, task.height, task.qfactor, task.color2Gray);
372+
task->width, task->height, task->qfactor, task->color2Gray);
373373
int ret =
374-
jpeg_get(task.width, task.height, task.qfactor, task.color2Gray, &jpeg);
374+
jpeg_get(task->width, task->height, task->qfactor, task->color2Gray, &jpeg);
375375
if (ret) {
376376
HAL_DANGER("server", "Failed to receive a JPEG snapshot...\n");
377377
static char response[] =
378378
"HTTP/1.1 503 Internal Error\r\n"
379379
"Connection: close\r\n\r\n";
380-
send_and_close(task.client_fd, response, sizeof(response) - 1); // zero ending string!
380+
send_and_close(task->client_fd, response, sizeof(response) - 1); // zero ending string!
381+
free(task);
381382
return NULL;
382383
}
383384
HAL_INFO("server", "JPEG snapshot has been received!\n");
@@ -388,11 +389,12 @@ void *send_jpeg_thread(void *vargp) {
388389
"Content-Length: %lu\r\n"
389390
"Connection: close\r\n\r\n",
390391
jpeg.jpegSize);
391-
send_to_fd(task.client_fd, buf, buf_len);
392-
send_to_fd(task.client_fd, jpeg.data, jpeg.jpegSize);
393-
send_to_fd(task.client_fd, "\r\n", 2);
394-
close_socket_fd(task.client_fd);
392+
send_to_fd(task->client_fd, buf, buf_len);
393+
send_to_fd(task->client_fd, jpeg.data, jpeg.jpegSize);
394+
send_to_fd(task->client_fd, "\r\n", 2);
395+
close_socket_fd(task->client_fd);
395396
free(jpeg.data);
397+
free(task);
396398
HAL_INFO("server", "JPEG snapshot has been sent!\n");
397399
return NULL;
398400
}
@@ -512,7 +514,13 @@ void parse_request(http_request_t *req) {
512514
if (req->query = strchr(req->uri, '?'))
513515
*req->query++ = '\0';
514516
else
515-
req->query = req->uri - 1;
517+
req->query = NULL;
518+
519+
if (req->uri && strstr(req->uri, "..")) {
520+
close_socket_fd(req->clntFd);
521+
req->clntFd = -1;
522+
return;
523+
}
516524

517525
http_header_t *h = http_headers;
518526
while (h < http_headers + 16) {
@@ -761,14 +769,14 @@ void respond_request(http_request_t *req) {
761769

762770
if (app_config.jpeg_enable && STARTS_WITH(req->uri, "/image.jpg")) {
763771
{
764-
struct jpegtask task;
765-
task.client_fd = req->clntFd;
766-
task.width = app_config.jpeg_width;
767-
task.height = app_config.jpeg_height;
768-
task.qfactor = app_config.jpeg_qfactor;
769-
task.color2Gray = 0;
770-
771-
if (!EMPTY(req->query)) {
772+
struct jpegtask *task = malloc(sizeof(struct jpegtask));
773+
task->client_fd = req->clntFd;
774+
task->width = app_config.jpeg_width;
775+
task->height = app_config.jpeg_height;
776+
task->qfactor = app_config.jpeg_qfactor;
777+
task->color2Gray = 0;
778+
779+
if (req->query) {
772780
char *remain;
773781
while (req->query) {
774782
char *value = split(&req->query, "&");
@@ -778,23 +786,23 @@ void respond_request(http_request_t *req) {
778786
if (EQUALS(key, "width")) {
779787
short result = strtol(value, &remain, 10);
780788
if (remain != value)
781-
task.width = result;
789+
task->width = result;
782790
}
783791
else if (EQUALS(key, "height")) {
784792
short result = strtol(value, &remain, 10);
785793
if (remain != value)
786-
task.height = result;
794+
task->height = result;
787795
}
788796
else if (EQUALS(key, "qfactor")) {
789797
short result = strtol(value, &remain, 10);
790798
if (remain != value)
791-
task.qfactor = result;
799+
task->qfactor = result;
792800
}
793801
else if (EQUALS(key, "color2gray") || EQUALS(key, "gray")) {
794802
if (EQUALS_CASE(value, "true") || EQUALS(value, "1"))
795-
task.color2Gray = 1;
803+
task->color2Gray = 1;
796804
else if (EQUALS_CASE(value, "false") || EQUALS(value, "0"))
797-
task.color2Gray = 0;
805+
task->color2Gray = 0;
798806
}
799807
}
800808
}
@@ -807,17 +815,21 @@ void respond_request(http_request_t *req) {
807815
size_t new_stacksize = 16 * 1024;
808816
if (pthread_attr_setstacksize(&thread_attr, new_stacksize))
809817
HAL_DANGER("jpeg", "Can't set stack size %zu\n", new_stacksize);
810-
pthread_create(
811-
&thread_id, &thread_attr, send_jpeg_thread, (void *)&task);
818+
if (pthread_create(
819+
&thread_id, &thread_attr, send_jpeg_thread, (void *)task)) {
820+
HAL_DANGER("jpeg", "Can't create thread\n");
821+
free(task);
822+
}
812823
if (pthread_attr_setstacksize(&thread_attr, stacksize))
813824
HAL_DANGER("jpeg", "Can't set stack size %zu\n", stacksize);
814825
pthread_attr_destroy(&thread_attr);
826+
pthread_detach(thread_id);
815827
}
816828
return;
817829
}
818830

819831
if (EQUALS(req->uri, "/api/audio")) {
820-
if (!EMPTY(req->query)) {
832+
if (req->query) {
821833
char *remain;
822834
while (req->query) {
823835
char *value = split(&req->query, "&");
@@ -863,7 +875,7 @@ void respond_request(http_request_t *req) {
863875

864876
if (EQUALS(req->uri, "/api/cmd")) {
865877
int result = -1;
866-
if (!EMPTY(req->query)) {
878+
if (req->query) {
867879
char *remain;
868880
while (req->query) {
869881
char *value = split(&req->query, "&");
@@ -897,7 +909,7 @@ void respond_request(http_request_t *req) {
897909
}
898910

899911
if (EQUALS(req->uri, "/api/isp")) {
900-
if (!EMPTY(req->query)) {
912+
if (req->query) {
901913
char *remain;
902914
while (req->query) {
903915
char *value = split(&req->query, "&");
@@ -933,7 +945,7 @@ void respond_request(http_request_t *req) {
933945
}
934946

935947
if (EQUALS(req->uri, "/api/jpeg")) {
936-
if (!EMPTY(req->query)) {
948+
if (req->query) {
937949
char *remain;
938950
while (req->query) {
939951
char *value = split(&req->query, "&");
@@ -973,7 +985,7 @@ void respond_request(http_request_t *req) {
973985
}
974986

975987
if (EQUALS(req->uri, "/api/mjpeg")) {
976-
if (!EMPTY(req->query)) {
988+
if (req->query) {
977989
char *remain;
978990
while (req->query) {
979991
char *value = split(&req->query, "&");
@@ -1041,7 +1053,7 @@ void respond_request(http_request_t *req) {
10411053
}
10421054

10431055
if (EQUALS(req->uri, "/api/mp4")) {
1044-
if (!EMPTY(req->query)) {
1056+
if (req->query) {
10451057
char *remain;
10461058
while (req->query) {
10471059
char *value = split(&req->query, "&");
@@ -1131,7 +1143,7 @@ void respond_request(http_request_t *req) {
11311143
}
11321144

11331145
if (EQUALS(req->uri, "/api/night")) {
1134-
if (!EMPTY(req->query)) {
1146+
if (req->query) {
11351147
char *remain;
11361148
while (req->query) {
11371149
char *value = split(&req->query, "&");
@@ -1254,8 +1266,7 @@ void respond_request(http_request_t *req) {
12541266
return;
12551267
}
12561268
}
1257-
if (!EMPTY(req->query))
1258-
{
1269+
if (req->query) {
12591270
char *remain;
12601271
while (req->query) {
12611272
char *value = split(&req->query, "&");
@@ -1334,7 +1345,7 @@ void respond_request(http_request_t *req) {
13341345
}
13351346

13361347
if (EQUALS(req->uri, "/api/record")) {
1337-
if (!EMPTY(req->query)) {
1348+
if (req->query) {
13381349
char *remain;
13391350
while (req->query) {
13401351
char *value = split(&req->query, "&");
@@ -1423,7 +1434,7 @@ void respond_request(http_request_t *req) {
14231434

14241435
if (EQUALS(req->uri, "/api/time")) {
14251436
struct timespec t;
1426-
if (!EMPTY(req->query)) {
1437+
if (req->query) {
14271438
char *remain;
14281439
while (req->query) {
14291440
char *value = split(&req->query, "&");

0 commit comments

Comments
 (0)