Widespread TOCTTOU

[guijan]

The code is littered with TOCTTOU issues.

These need a condition handler around rename-file and delete-file to handle implementations that raise file-error if there is an error, the solution isn't probe-file because of the inherent TOCTTOU in probe-file:

quicklisp-client/quicklisp/utils.lisp

Lines 28 to 32 in 10b61e5

	(defun replace-file (from to)
	"Like RENAME-FILE, but deletes TO if it exists, first."
	(when (probe-file to)
	(delete-file to))
	(rename-file from to))

quicklisp-client/quicklisp/utils.lisp

Lines 55 to 57 in 10b61e5

	(defun delete-file-if-exists (pathname)
	(when (probe-file pathname)
	(delete-file pathname)))

This probe-file in copy-file seems completely unneeded:

quicklisp-client/quicklisp/utils.lisp

Line 50 in 10b61e5

(probe-file to)))

Also, it appears there's a duplicate version of copy-file:

quicklisp-client/quicklisp/bundle.lisp

Lines 270 to 283 in 10b61e5

	(defun copy-file (from-file to-file)
	(with-open-file (from-stream from-file :element-type '(unsigned-byte 8)
	:if-does-not-exist nil)
	(when from-stream
	(let ((buffer (make-array 10000 :element-type '(unsigned-byte 8))))
	(with-open-file (to-stream to-file
	:direction :output
	:if-exists :supersede
	:element-type '(unsigned-byte 8))
	(loop
	(let ((end-index (read-sequence buffer from-stream)))
	(when (zerop end-index)
	(return to-file))
	(write-sequence buffer to-stream :end end-index))))))))

I sent a PR with an example: #233

If such fixes are acceptable, I'll work on this issue.

[quicklisp]

I'm open to fixes like from #223 - I didn't even know LOAD had a keyword like that, so that's a nice solution. For other stuff, it depends on how complicated it gets. The probe-file in the first copy-file could probably be a truename instead. The duplicate copy-file is probably unintentional.

…

On Fri, Nov 22, 2024 at 10:53 PM Guilherme Janczak ***@***.***> wrote: The code is littered with TOCTTOU issues. These need a condition handler around rename-file and delete-file to handle implementations that raise file-error if there is an error, the solution isn't probe-file because of the inherent TOCTTOU in probe-file: https://github.com/quicklisp/quicklisp-client/blob/10b61e5220ba20bfdd88c1086d2523bd29414a8b/quicklisp/utils.lisp#L28-L32 https://github.com/quicklisp/quicklisp-client/blob/10b61e5220ba20bfdd88c1086d2523bd29414a8b/quicklisp/utils.lisp#L55-L57 This probe-file seems completely unneeded https://github.com/quicklisp/quicklisp-client/blob/10b61e5220ba20bfdd88c1086d2523bd29414a8b/quicklisp/utils.lisp#L50 Also, it appears there's a duplicate version of copy-file: https://github.com/quicklisp/quicklisp-client/blob/10b61e5220ba20bfdd88c1086d2523bd29414a8b/quicklisp/bundle.lisp#L270-L283 I sent a PR with an example: #233 <#233> If such fixes are acceptable, I'll work on this issue. — Reply to this email directly, view it on GitHub <#234>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACPNLLJTAC6JWJJL6O2KP32B7343AVCNFSM6AAAAABSKTYG2CVHI2DSMVQWIX3LMV43ASLTON2WKOZSGY4DKMRYGI2DKNQ> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>