ftpd: close the data socket and the file on every exit path
Route every close of the data socket through close_data_sock so that the number is reset to -1 afterwards, and add close_pasv_sock for the passive listener. PASV and PORT now drop the data connection left behind by the previous PASV/PORT, QUIT closes both the data socket and a pending listener, and the 550 paths of LIST/RETR close the data socket the client already connected. A failed file.read/file.write, and a failed send/recv mid-transfer (transfer_error), now close the file and the data socket instead of leaking both. Verified in QEMU with a scripted FTP client: LIST/RETR/STOR roundtrip, PASV after PASV, PORT after PASV, RETR of a missing file, STOR with an over-long name, 24 LIST cycles with stray data connections, QUIT. Assisted-by: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
1 parent
fc9aabc663
commit
4982652b1d
1 file changed
+45
-21
@@ -139,7 +139,7 @@ socketerror:
|
||||
align 4
|
||||
; Close the data socket if this thread currently owns one. datasocketnum only
|
||||
; holds a live socket in these two modes; in any other mode it is a handle that
|
||||
; has already been closed and may since have been reused by another socket.
|
||||
; has already been closed or was never opened.
|
||||
close_data_sock:
|
||||
cmp [ebp + thread_data.mode], MODE_PASSIVE_OK
|
||||
je @f
|
||||
@@ -152,13 +152,32 @@ close_data_sock:
|
||||
.done:
|
||||
ret
|
||||
|
||||
align 4
|
||||
; Close the passive listening socket if one is still open. It is open only
|
||||
; while waiting for the client (MODE_PASSIVE_WAIT, MODE_PASSIVE_FAILED) or after
|
||||
; a failed listen, so no data connection exists and the mode drops to NOTREADY.
|
||||
close_pasv_sock:
|
||||
cmp [ebp + thread_data.passivesocknum], -1
|
||||
je .done
|
||||
mcall close, [ebp + thread_data.passivesocknum]
|
||||
mov [ebp + thread_data.passivesocknum], -1
|
||||
mov [ebp + thread_data.mode], MODE_NOTREADY
|
||||
.done:
|
||||
ret
|
||||
|
||||
align 4
|
||||
; A data transfer failed with the file in ebx open: close the file and the data
|
||||
; socket, then report the error.
|
||||
transfer_error:
|
||||
invoke file.close, ebx
|
||||
call close_data_sock
|
||||
jmp socketerror
|
||||
|
||||
align 4
|
||||
abort_transfer:
|
||||
and [ebp + thread_data.permissions], not ABORT
|
||||
mov [ebp + thread_data.mode], MODE_NOTREADY
|
||||
invoke file.close, ebx
|
||||
mcall close, [ebp + thread_data.datasocketnum]
|
||||
mov [ebp + thread_data.datasocketnum], -1
|
||||
call close_data_sock
|
||||
|
||||
sendFTP "530 Transfer aborted"
|
||||
ret
|
||||
@@ -708,13 +727,13 @@ cmdLIST:
|
||||
mcall send
|
||||
|
||||
; close the data socket..
|
||||
mov [ebp + thread_data.mode], MODE_NOTREADY
|
||||
mcall close, [ebp + thread_data.datasocketnum]
|
||||
call close_data_sock
|
||||
|
||||
sendFTP "226 List OK"
|
||||
ret
|
||||
|
||||
.nosuchdir:
|
||||
call close_data_sock
|
||||
sendFTP "550 Directory does not exist"
|
||||
ret
|
||||
|
||||
@@ -810,11 +829,9 @@ cmdPASS:
|
||||
align 4
|
||||
cmdPASV:
|
||||
|
||||
cmp [ebp + thread_data.passivesocknum], -1
|
||||
je @f
|
||||
mcall close, [ebp + thread_data.passivesocknum] ; if there is still a socket open, close it
|
||||
mov [ebp + thread_data.passivesocknum], -1
|
||||
@@:
|
||||
; Drop whatever data connection the previous PASV/PORT left behind
|
||||
call close_data_sock
|
||||
call close_pasv_sock
|
||||
|
||||
; Open a new TCP socket
|
||||
mcall socket, AF_INET4, SOCK_STREAM, 0
|
||||
@@ -950,6 +967,10 @@ cmdPWD:
|
||||
align 4
|
||||
cmdPORT:
|
||||
|
||||
; Drop whatever data connection the previous PASV/PORT left behind
|
||||
call close_data_sock
|
||||
call close_pasv_sock
|
||||
|
||||
; PORT a1,a2,a3,a4,p1,p2
|
||||
; IP address a1.a2.a3.a4, port p1*256+p2
|
||||
|
||||
@@ -990,7 +1011,8 @@ align 4
|
||||
cmdQUIT:
|
||||
|
||||
sendFTP "221 Bye!"
|
||||
mcall close, [ebp + thread_data.datasocketnum]
|
||||
call close_data_sock
|
||||
call close_pasv_sock
|
||||
mcall close, [ebp + thread_data.socketnum]
|
||||
|
||||
add esp, 4 ; get rid of call return address
|
||||
@@ -1053,7 +1075,7 @@ cmdRETR:
|
||||
lea eax, [ebp + thread_data.buffer] ; FIXME: use another buffer!! if we receive something on control connection now, we screw up!
|
||||
invoke file.read, ebx, eax, BUFFERSIZE
|
||||
cmp eax, -1
|
||||
je .cannot_open ; FIXME: this is not the correct error
|
||||
je .read_error ; FIXME: this is not the correct error
|
||||
|
||||
push eax
|
||||
invoke con_write_asciiz, str2
|
||||
@@ -1067,7 +1089,7 @@ cmdRETR:
|
||||
mcall send
|
||||
pop ebx ecx
|
||||
cmp eax, -1
|
||||
je socketerror ; FIXME: not the correct error
|
||||
je transfer_error ; FIXME: not the correct error
|
||||
|
||||
; cmp eax, ecx
|
||||
; jne not_all_byes_sent ; TODO
|
||||
@@ -1079,13 +1101,15 @@ cmdRETR:
|
||||
|
||||
invoke con_write_asciiz, str2b
|
||||
|
||||
mov [ebp + thread_data.mode], MODE_NOTREADY
|
||||
mcall close, [ebp + thread_data.datasocketnum]
|
||||
call close_data_sock
|
||||
|
||||
sendFTP "226 Transfer OK, closing connection"
|
||||
ret
|
||||
|
||||
.read_error:
|
||||
invoke file.close, ebx
|
||||
.cannot_open:
|
||||
call close_data_sock
|
||||
invoke con_set_flags, 0x0c
|
||||
invoke con_write_asciiz, str_notfound
|
||||
invoke con_set_flags, 0x07
|
||||
@@ -1167,7 +1191,7 @@ cmdSTOR:
|
||||
mcall recv
|
||||
pop ebx ecx
|
||||
cmp eax, -1
|
||||
je socketerror ; FIXME: not the correct error
|
||||
je transfer_error ; FIXME: not the correct error
|
||||
|
||||
test eax, eax
|
||||
jz @f
|
||||
@@ -1179,7 +1203,7 @@ cmdSTOR:
|
||||
pop edx
|
||||
|
||||
cmp eax, -1
|
||||
je .cannot_open ; FIXME: this is not the correct error
|
||||
je .write_error ; FIXME: this is not the correct error
|
||||
|
||||
invoke con_write_asciiz, str2
|
||||
|
||||
@@ -1195,9 +1219,7 @@ cmdSTOR:
|
||||
|
||||
invoke con_write_asciiz, str2b
|
||||
|
||||
mov [ebp + thread_data.mode], MODE_NOTREADY
|
||||
mcall close, [ebp + thread_data.datasocketnum]
|
||||
mov [ebp + thread_data.datasocketnum], -1
|
||||
call close_data_sock
|
||||
|
||||
|
||||
|
||||
@@ -1211,6 +1233,8 @@ cmdSTOR:
|
||||
sendFTP "226 Transfer OK"
|
||||
ret
|
||||
|
||||
.write_error:
|
||||
invoke file.close, ebx
|
||||
.cannot_open:
|
||||
call close_data_sock
|
||||
sendFTP "550 No create file"
|
||||
|
||||
Reference in new issue
Block a user