search

quicklisp / quicklisp-client

Quicklisp client.
http://www.quicklisp.org/
MIT License
298 stars 75 forks source link

Widespread TOCTTOU #234

Open guijan opened 3 days ago

guijan commented 3 days ago

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 in copy-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: https://github.com/quicklisp/quicklisp-client/pull/233

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

quicklisp commented 1 day ago

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 https://github.com/quicklisp/quicklisp-client/pull/233

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

— Reply to this email directly, view it on GitHub https://github.com/quicklisp/quicklisp-client/issues/234, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACPNLLJTAC6JWJJL6O2KP32B7343AVCNFSM6AAAAABSKTYG2CVHI2DSMVQWIX3LMV43ASLTON2WKOZSGY4DKMRYGI2DKNQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>