Commit 9367964ae21d2a41517c76c87dd26c79abcfe937
Committed by
Anthony Liguori
1 parent
20c24bf2
slirp: tftp: Rework filename handling
This changes the filename handling from a static buffer in tftp_session for the client-provided name + prefix to a dynamically allocated buffer that keeps the combined path in one place. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Showing
1 changed file
with
16 additions
and
22 deletions
slirp/tftp.c
| @@ -27,7 +27,7 @@ | @@ -27,7 +27,7 @@ | ||
| 27 | 27 | ||
| 28 | struct tftp_session { | 28 | struct tftp_session { |
| 29 | int in_use; | 29 | int in_use; |
| 30 | - char filename[TFTP_FILENAME_MAX]; | 30 | + char *filename; |
| 31 | 31 | ||
| 32 | struct in_addr client_ip; | 32 | struct in_addr client_ip; |
| 33 | u_int16_t client_port; | 33 | u_int16_t client_port; |
| @@ -47,6 +47,7 @@ static void tftp_session_update(struct tftp_session *spt) | @@ -47,6 +47,7 @@ static void tftp_session_update(struct tftp_session *spt) | ||
| 47 | 47 | ||
| 48 | static void tftp_session_terminate(struct tftp_session *spt) | 48 | static void tftp_session_terminate(struct tftp_session *spt) |
| 49 | { | 49 | { |
| 50 | + qemu_free(spt->filename); | ||
| 50 | spt->in_use = 0; | 51 | spt->in_use = 0; |
| 51 | } | 52 | } |
| 52 | 53 | ||
| @@ -62,8 +63,10 @@ static int tftp_session_allocate(struct tftp_t *tp) | @@ -62,8 +63,10 @@ static int tftp_session_allocate(struct tftp_t *tp) | ||
| 62 | goto found; | 63 | goto found; |
| 63 | 64 | ||
| 64 | /* sessions time out after 5 inactive seconds */ | 65 | /* sessions time out after 5 inactive seconds */ |
| 65 | - if ((int)(curtime - spt->timestamp) > 5000) | 66 | + if ((int)(curtime - spt->timestamp) > 5000) { |
| 67 | + qemu_free(spt->filename); | ||
| 66 | goto found; | 68 | goto found; |
| 69 | + } | ||
| 67 | } | 70 | } |
| 68 | 71 | ||
| 69 | return -1; | 72 | return -1; |
| @@ -103,15 +106,8 @@ static int tftp_read_data(struct tftp_session *spt, u_int16_t block_nr, | @@ -103,15 +106,8 @@ static int tftp_read_data(struct tftp_session *spt, u_int16_t block_nr, | ||
| 103 | { | 106 | { |
| 104 | int fd; | 107 | int fd; |
| 105 | int bytes_read = 0; | 108 | int bytes_read = 0; |
| 106 | - char buffer[1024]; | ||
| 107 | - int n; | ||
| 108 | - | ||
| 109 | - n = snprintf(buffer, sizeof(buffer), "%s/%s", | ||
| 110 | - tftp_prefix, spt->filename); | ||
| 111 | - if (n >= sizeof(buffer)) | ||
| 112 | - return -1; | ||
| 113 | 109 | ||
| 114 | - fd = open(buffer, O_RDONLY | O_BINARY); | 110 | + fd = open(spt->filename, O_RDONLY | O_BINARY); |
| 115 | 111 | ||
| 116 | if (fd < 0) { | 112 | if (fd < 0) { |
| 117 | return -1; | 113 | return -1; |
| @@ -274,6 +270,7 @@ static void tftp_handle_rrq(struct tftp_t *tp, int pktlen) | @@ -274,6 +270,7 @@ static void tftp_handle_rrq(struct tftp_t *tp, int pktlen) | ||
| 274 | { | 270 | { |
| 275 | struct tftp_session *spt; | 271 | struct tftp_session *spt; |
| 276 | int s, k; | 272 | int s, k; |
| 273 | + size_t prefix_len; | ||
| 277 | char *req_fname; | 274 | char *req_fname; |
| 278 | 275 | ||
| 279 | s = tftp_session_allocate(tp); | 276 | s = tftp_session_allocate(tp); |
| @@ -294,8 +291,13 @@ static void tftp_handle_rrq(struct tftp_t *tp, int pktlen) | @@ -294,8 +291,13 @@ static void tftp_handle_rrq(struct tftp_t *tp, int pktlen) | ||
| 294 | k = 0; | 291 | k = 0; |
| 295 | pktlen -= ((uint8_t *)&tp->x.tp_buf[0] - (uint8_t *)tp); | 292 | pktlen -= ((uint8_t *)&tp->x.tp_buf[0] - (uint8_t *)tp); |
| 296 | 293 | ||
| 294 | + /* prepend tftp_prefix */ | ||
| 295 | + prefix_len = strlen(tftp_prefix); | ||
| 296 | + spt->filename = qemu_malloc(prefix_len + TFTP_FILENAME_MAX + 1); | ||
| 297 | + memcpy(spt->filename, tftp_prefix, prefix_len); | ||
| 298 | + | ||
| 297 | /* get name */ | 299 | /* get name */ |
| 298 | - req_fname = spt->filename; | 300 | + req_fname = spt->filename + prefix_len; |
| 299 | 301 | ||
| 300 | while (1) { | 302 | while (1) { |
| 301 | if (k >= TFTP_FILENAME_MAX || k >= pktlen) { | 303 | if (k >= TFTP_FILENAME_MAX || k >= pktlen) { |
| @@ -322,10 +324,8 @@ static void tftp_handle_rrq(struct tftp_t *tp, int pktlen) | @@ -322,10 +324,8 @@ static void tftp_handle_rrq(struct tftp_t *tp, int pktlen) | ||
| 322 | k += 6; /* skipping octet */ | 324 | k += 6; /* skipping octet */ |
| 323 | 325 | ||
| 324 | /* do sanity checks on the filename */ | 326 | /* do sanity checks on the filename */ |
| 325 | - | ||
| 326 | - if ((spt->filename[0] != '/') | ||
| 327 | - || (spt->filename[strlen((char *)spt->filename) - 1] == '/') | ||
| 328 | - || strstr((char *)spt->filename, "/../")) { | 327 | + if (req_fname[0] != '/' || req_fname[strlen(req_fname) - 1] == '/' || |
| 328 | + strstr(req_fname, "/../")) { | ||
| 329 | tftp_send_error(spt, 2, "Access violation", tp); | 329 | tftp_send_error(spt, 2, "Access violation", tp); |
| 330 | return; | 330 | return; |
| 331 | } | 331 | } |
| @@ -360,13 +360,7 @@ static void tftp_handle_rrq(struct tftp_t *tp, int pktlen) | @@ -360,13 +360,7 @@ static void tftp_handle_rrq(struct tftp_t *tp, int pktlen) | ||
| 360 | struct stat stat_p; | 360 | struct stat stat_p; |
| 361 | 361 | ||
| 362 | if (tsize == 0) { | 362 | if (tsize == 0) { |
| 363 | - char buffer[1024]; | ||
| 364 | - int len; | ||
| 365 | - | ||
| 366 | - len = snprintf(buffer, sizeof(buffer), "%s/%s", | ||
| 367 | - tftp_prefix, spt->filename); | ||
| 368 | - | ||
| 369 | - if (stat(buffer, &stat_p) == 0) | 363 | + if (stat(spt->filename, &stat_p) == 0) |
| 370 | tsize = stat_p.st_size; | 364 | tsize = stat_p.st_size; |
| 371 | else { | 365 | else { |
| 372 | tftp_send_error(spt, 1, "File not found", tp); | 366 | tftp_send_error(spt, 1, "File not found", tp); |