LGTM.com - false positive for cpp/toctou-race-condition

[eldering]

Description of the false positive

Even though the code first does a stat and then after that an open on the same file, it still afterwards checks that the open call succeeded before operating on the file descriptor. Thus, there is no toctou race condition. I understand that this can be considered an edge case, but still reporting it here for consideration.

URL to the alert on the project page on LGTM.com

https://lgtm.com/projects/g/DOMjudge/domjudge/snapshot/66789bbc091123b7a00dc368032b9dbb5f3e0e96/files/judge/evict.c?sort=name&dir=ASC&mode=heatmap#x5362a542144696a7:1

[geoffw0]

Hi, thanks for raising this.

We agree that checking the return value of open covers the case that the file no longer exists, however we believe there is still a potential TOCTOU issue involving the path to a file that's replaced with a directory. In this case the S_ISDIR check could be fooled and a directory unexpectedly passed to posix_fadvise. We're suspicious this wouldn't be harmful but that's beyond the scope of this query.

We'll be keeping an eye on this query, and adding some new test cases in the near future.

I suggest that you double check you're happy with the code in evict_directory, and then hide the result in the LGTM interface. Let us know if you have any further questions or concerns.

[eldering]

Thanks, that's a correct point that I overlooked. Should be fixed with my PR DOMjudge/domjudge#1249.