Skip to content

Commit fc69675

Browse files
starnightstarnight
committed
Add URI and HTTP version length boundary to avoid buffer overflow
Fixes: #8
1 parent 415ae03 commit fc69675

6 files changed

Lines changed: 120 additions & 36 deletions

File tree

‎FreeRTOS/lib/middleware.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ int AddRoute(HTTPMethod method, char *uri, SAF saf) {
3434
#if ENABLE_STATIC_FILE == 1
3535

3636
#ifndef MAX_PATH_SIZE
37-
#define MAX_PATH_SIZE 128
37+
#define MAX_PATH_SIZE (sizeof(STATIC_FILE_FOLDER) + MAX_HTTP_URI_LEN)
3838
#endif
3939

4040
/* Try to read static files under static folder. */
@@ -53,7 +53,7 @@ uint8_t _ReadStaticFiles(HTTPReqMessage *req, HTTPResMessage *res) {
5353
"Content-Type: text/html; charset=UTF-8\r\n\r\n";
5454

5555
/* Prevent path buffer overflow. */
56-
if (strlen(STATIC_FILE_FOLDER) + n >= MAX_PATH_SIZE)
56+
if (n >= MAX_HTTP_URI_LEN)
5757
return found;
5858

5959
/* Prevent Path Traversal. */

‎FreeRTOS/lib/server.c‎

Lines changed: 54 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,8 @@ int _ParseHeader(HTTPReq *hr) {
192192
DebugMsg("\tParse Header\n");
193193
p = (char *)req->_buf;
194194
memset(p, 0, MAX_HEADER_SIZE + MAX_BODY_SIZE);
195+
req->_index = 0;
196+
195197
/* GET, PUT ... and a white space are 3 charaters. */
196198
n = recv(clisock, p, 3, 0);
197199
if(n == 3) {
@@ -209,6 +211,7 @@ int _ParseHeader(HTTPReq *hr) {
209211
if(p[i] == ' ') {
210212
p[i] = '\0';
211213
i += 1;
214+
req->_index = i;
212215
break;
213216
}
214217

@@ -218,34 +221,67 @@ int _ParseHeader(HTTPReq *hr) {
218221
req->Header.Method = HaveMethod(p);
219222

220223
/* Parse URI. */
221-
req->Header.URI = p + i;
222-
for(; n>0; i++) {
224+
p += i;
225+
req->Header.URI = p;
226+
for(i = 0; n>0; i++) {
223227
n = recv(clisock, p + i, 1, 0);
228+
if(n == -EAGAIN || n == -EWOULDBLOCK) {
229+
n = 1;
230+
i--;
231+
continue;
232+
} else if (n <= 0) {
233+
return 0;
234+
}
235+
224236
if(p[i] == ' ') {
225237
p[i] = '\0';
238+
i += 1;
239+
req->_index += i;
226240
break;
227241
}
242+
243+
if(i > MAX_HTTP_URI_LEN - 2) {
244+
strncpy(req->Header.URI, "notfound", 9);
245+
req->_index += 9;
246+
return req->_index;
247+
}
228248
}
229249

230250
/* Parse HTTP version. */
231-
if(n > 0) i += 1;
232-
req->Header.Version = p + i;
233-
/* HTTP/1.1 has 8 charaters. */
234-
n = recv(clisock, p + i, 8, 0);
235-
for(i+=8; (n>0) && (i<MAX_HEADER_SIZE); i++) {
251+
p += i;
252+
req->Header.Version = p;
253+
for(i = 0; n>0; i++) {
236254
n = recv(clisock, p + i, 1, 0);
237-
if((l = _CheckLine(p + i))) {
238-
if(l == 2) p[i - 1] = '\0';
255+
if(n == -EAGAIN || n == -EWOULDBLOCK) {
256+
n = 1;
257+
i--;
258+
continue;
259+
} else if (n <= 0) {
260+
return 0;
261+
}
262+
263+
/* HTTP/1.1 has 8 charaters */
264+
if(i >= 8 && (l = _CheckLine(p + i))) {
239265
p[i] = '\0';
266+
i += 1;
267+
if(l == 2) {
268+
p[i - 1] = '\0';
269+
i -= 1;
270+
}
271+
req->_index += i;
240272
break;
241273
}
274+
275+
/* 8 charaters adds 2 characters for break line */
276+
if(i > 8 + 2 - 2)
277+
return 0;
242278
}
243279

244280
/* Parse other fields. */
245-
if(n > 0) i += 1;
246-
req->Header.Fields[req->Header.Amount].key = p + i;
281+
p += i;
282+
req->Header.Fields[req->Header.Amount].key = p;
247283
end = 0;
248-
for(; (n>0) && (i<MAX_HEADER_SIZE) && (req->Header.Amount<MAX_HEADER_FIELDS); i++) {
284+
for(i = 0; (n>0) && (req->_index+i < MAX_HEADER_SIZE) && (req->Header.Amount < MAX_HEADER_FIELDS); i++) {
249285
n = recv(clisock, p + i, 1, 0);
250286
/* Check field key name end. */
251287
if((l = _CheckFieldSep(p + i))) {
@@ -256,8 +292,11 @@ int _ParseHeader(HTTPReq *hr) {
256292
/* Check header end. */
257293
if((l = _CheckLine(p + i))) {
258294
if(end == 0) {
259-
if(l == 2) p[i - 1] = '\0';
260295
p[i] = '\0';
296+
if(l == 2) {
297+
p[i - 1] = '\0';
298+
i -= 1;
299+
}
261300

262301
/* CRLF have 2 characters, so check 2 times new line. */
263302
end = 2;
@@ -275,13 +314,13 @@ int _ParseHeader(HTTPReq *hr) {
275314
if(end > 0) end -= 1;
276315
}
277316
}
317+
req->_index += i;
278318
}
279319
if(n < 0) {
280320
hr->work_state = CLOSE_SOCKET;
281321
}
282322

283-
req->_index = (n > 0) ? i + 1: i;
284-
return i;
323+
return req->_index;
285324
}
286325

287326
int _IsLengthHeader(char *key) {

‎FreeRTOS/lib/server.h‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,12 @@
66
#include <unistd.h>
77

88
#define MAX_HEADER_SIZE 1024
9-
#define MAX_BODY_SIZE 1024
9+
#ifndef MAX_HTTP_URI_LEN
10+
#define MAX_HTTP_URI_LEN 128
11+
#endif
12+
#define MAX_BODY_SIZE 2048
1013
#ifndef MHS_PORT
11-
#define MHS_PORT 80
14+
#define MHS_PORT 8001
1215
#endif
1316
#ifndef MAX_HTTP_CLIENT
1417
#define MAX_HTTP_CLIENT 5

‎c-version/lib/middleware.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ int AddRoute(HTTPMethod method, char *uri, SAF saf) {
3434
#if ENABLE_STATIC_FILE == 1
3535

3636
#ifndef MAX_PATH_SIZE
37-
#define MAX_PATH_SIZE 128
37+
#define MAX_PATH_SIZE (sizeof(STATIC_FILE_FOLDER) + MAX_HTTP_URI_LEN)
3838
#endif
3939

4040
/* Try to read static files under static folder. */
@@ -53,7 +53,7 @@ uint8_t _ReadStaticFiles(HTTPReqMessage *req, HTTPResMessage *res) {
5353
"Content-Type: text/html; charset=UTF-8\r\n\r\n";
5454

5555
/* Prevent path buffer overflow. */
56-
if (strlen(STATIC_FILE_FOLDER) + n >= MAX_PATH_SIZE)
56+
if (n >= MAX_HTTP_URI_LEN)
5757
return found;
5858

5959
/* Prevent Path Traversal. */

‎c-version/lib/server.c‎

Lines changed: 54 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,8 @@ int _ParseHeader(HTTPReq *hr) {
192192
DebugMsg("\tParse Header\n");
193193
p = (char *)req->_buf;
194194
memset(p, 0, MAX_HEADER_SIZE + MAX_BODY_SIZE);
195+
req->_index = 0;
196+
195197
/* GET, PUT ... and a white space are 3 charaters. */
196198
n = recv(clisock, p, 3, 0);
197199
if(n == 3) {
@@ -209,6 +211,7 @@ int _ParseHeader(HTTPReq *hr) {
209211
if(p[i] == ' ') {
210212
p[i] = '\0';
211213
i += 1;
214+
req->_index = i;
212215
break;
213216
}
214217

@@ -218,34 +221,67 @@ int _ParseHeader(HTTPReq *hr) {
218221
req->Header.Method = HaveMethod(p);
219222

220223
/* Parse URI. */
221-
req->Header.URI = p + i;
222-
for(; n>0; i++) {
224+
p += i;
225+
req->Header.URI = p;
226+
for(i = 0; n>0; i++) {
223227
n = recv(clisock, p + i, 1, 0);
228+
if(n == -EAGAIN || n == -EWOULDBLOCK) {
229+
n = 1;
230+
i--;
231+
continue;
232+
} else if (n <= 0) {
233+
return 0;
234+
}
235+
224236
if(p[i] == ' ') {
225237
p[i] = '\0';
238+
i += 1;
239+
req->_index += i;
226240
break;
227241
}
242+
243+
if(i > MAX_HTTP_URI_LEN - 2) {
244+
strncpy(req->Header.URI, "notfound", 9);
245+
req->_index += 9;
246+
return req->_index;
247+
}
228248
}
229249

230250
/* Parse HTTP version. */
231-
if(n > 0) i += 1;
232-
req->Header.Version = p + i;
233-
/* HTTP/1.1 has 8 charaters. */
234-
n = recv(clisock, p + i, 8, 0);
235-
for(i+=8; (n>0) && (i<MAX_HEADER_SIZE); i++) {
251+
p += i;
252+
req->Header.Version = p;
253+
for(i = 0; n>0; i++) {
236254
n = recv(clisock, p + i, 1, 0);
237-
if((l = _CheckLine(p + i))) {
238-
if(l == 2) p[i - 1] = '\0';
255+
if(n == -EAGAIN || n == -EWOULDBLOCK) {
256+
n = 1;
257+
i--;
258+
continue;
259+
} else if (n <= 0) {
260+
return 0;
261+
}
262+
263+
/* HTTP/1.1 has 8 charaters */
264+
if(i >= 8 && (l = _CheckLine(p + i))) {
239265
p[i] = '\0';
266+
i += 1;
267+
if(l == 2) {
268+
p[i - 1] = '\0';
269+
i -= 1;
270+
}
271+
req->_index += i;
240272
break;
241273
}
274+
275+
/* 8 charaters adds 2 characters for break line */
276+
if(i > 8 + 2 - 2)
277+
return 0;
242278
}
243279

244280
/* Parse other fields. */
245-
if(n > 0) i += 1;
246-
req->Header.Fields[req->Header.Amount].key = p + i;
281+
p += i;
282+
req->Header.Fields[req->Header.Amount].key = p;
247283
end = 0;
248-
for(; (n>0) && (i<MAX_HEADER_SIZE) && (req->Header.Amount<MAX_HEADER_FIELDS); i++) {
284+
for(i = 0; (n>0) && (req->_index+i < MAX_HEADER_SIZE) && (req->Header.Amount < MAX_HEADER_FIELDS); i++) {
249285
n = recv(clisock, p + i, 1, 0);
250286
/* Check field key name end. */
251287
if((l = _CheckFieldSep(p + i))) {
@@ -256,8 +292,11 @@ int _ParseHeader(HTTPReq *hr) {
256292
/* Check header end. */
257293
if((l = _CheckLine(p + i))) {
258294
if(end == 0) {
259-
if(l == 2) p[i - 1] = '\0';
260295
p[i] = '\0';
296+
if(l == 2) {
297+
p[i - 1] = '\0';
298+
i -= 1;
299+
}
261300

262301
/* CRLF have 2 characters, so check 2 times new line. */
263302
end = 2;
@@ -275,13 +314,13 @@ int _ParseHeader(HTTPReq *hr) {
275314
if(end > 0) end -= 1;
276315
}
277316
}
317+
req->_index += i;
278318
}
279319
if(n < 0) {
280320
hr->work_state = CLOSE_SOCKET;
281321
}
282322

283-
req->_index = (n > 0) ? i + 1: i;
284-
return i;
323+
return req->_index;
285324
}
286325

287326
int _IsLengthHeader(char *key) {

‎c-version/lib/server.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@
66
#include <unistd.h>
77

88
#define MAX_HEADER_SIZE 1024
9+
#ifndef MAX_HTTP_URI_LEN
10+
#define MAX_HTTP_URI_LEN 128
11+
#endif
912
#define MAX_BODY_SIZE 2048
1013
#ifndef MHS_PORT
1114
#define MHS_PORT 8001

0 commit comments

Comments
 (0)