add length validation for Lock-Token to prevent underflow#618
add length validation for Lock-Token to prevent underflow#618metsw24-max wants to merge 9 commits intoapache:trunkfrom
Conversation
|
Ensure safe handling of Lock-Token headers by validating string length Guard access to This change improves robustness and establishes a safer parsing pattern |
|
This mod_dav.c change doesn't compile, so it is not obvious that you are "improving robustness" here. |
|
@notroj I have fixed the compilation issue and pushed an updated patch |
|
Seems like we now have a complex test duplicated across two places. How about making a single new function in dav/main/util.c which safely extracts the Lock-Token header, returns it stripped of <> via a char ** output pointer, and returns dav_error * for the error case |
|
@notroj This centralizes validation, ensures safe length checks, and avoids out-of-bounds access. The function returns a Both mod_dav.c and ms_wdv.c have been updated to use this helper removing duplication and aligning behavior across modules. |
|
I have implemented the helper-based refactor for Lock-Token parsing as discussed |
notroj
left a comment
There was a problem hiding this comment.
Thanks a lot for updating the PR, couple more bits of feedback here.
| return dav_new_error(p, HTTP_INTERNAL_SERVER_ERROR, 0, 0, | ||
| "DAV lock token parser called with NULL output pointer."); | ||
| } | ||
| *output = NULL; |
There was a problem hiding this comment.
This is unnecessary, we don't generally do argument validation in httpd
modules/dav/main/util.c
Outdated
| "Malformed Lock-Token header."); | ||
| } | ||
|
|
||
| token = apr_pstrndup(p, input + 1, len - 2); |
modules/dav/main/util.c
Outdated
|
|
||
| if (*token == '\0') { | ||
| return dav_new_error(p, HTTP_BAD_REQUEST, 0, 0, | ||
| "Malformed Lock-Token header."); |
There was a problem hiding this comment.
For both the error paths I'd say just use "Malformed Lock-Token" since it's used to parse headers as well.
|
-removed redundant validation |
This change improves the handling of the Lock-Token request header in mod_dav by adding proper length validation before performing string indexing and trimming operations
The existing implementation processes the
Lock-Tokenheader without verifying that the string length is sufficient before-Accessing
lock_token_hdr[len - 1]-Computing
len - 2forapr_pstrndup