schannel: fix error check logic in get_client_cert() file reader - #22415
Closed
vszakats wants to merge 1 commit into
Closed
schannel: fix error check logic in get_client_cert() file reader#22415vszakats wants to merge 1 commit into
get_client_cert() file reader#22415vszakats wants to merge 1 commit into
Conversation
"`ftell` returns `-1L` on error, but `cert_tell` is declared as `long` and initialized to `0`. If `fseek` fails, `continue_reading` is set to `FALSE` but `cert_tell` remains `0`, and on the next line `cert_tell < 0` would be false. More importantly, if `continue_reading` is false after `fseek`, `ftell` is not called, but the check `if(cert_tell < 0)` will still be evaluated. The check should be guarded: it should only be reached if `continue_reading` is still true, otherwise the `certsize` assignment on line 450 is skipped correctly but the negative check is misleadingly evaluated. Consider restructuring so `ftell` is only checked when `continue_reading` is true before calling it." Reported by GitHub Code Quality
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reported by GitHub Code Quality
Follow-up to 0fdf965 #5193
https://github.com/curl/curl/pull/22415/files?w=1
"
ftellreturns-1Lon error, butcert_tellis declared aslongand initialized to
0. Iffseekfails,continue_readingis set toFALSEbutcert_tellremains0, and on the next linecert_tell < 0would be false. More importantly, ifcontinue_readingis falseafter
fseek,ftellis not called, but the checkif(cert_tell < 0)will still be evaluated. The check should be guarded: it should only be
reached if
continue_readingis still true, otherwise thecertsizeassignment on line 450 is skipped correctly but the negative check is
misleadingly evaluated. Consider restructuring so
ftellis onlychecked when
continue_readingis true before calling it."