__readlink_chk, __readlinkat_chk: properly pass buffer length#115
__readlink_chk, __readlinkat_chk: properly pass buffer length#115yorickvP wants to merge 1 commit into
Conversation
josch
left a comment
josch
left a comment
There was a problem hiding this comment.
This effectively circumvents the check that is done in __readlink_chk which checks if (len > buflen) by passing FAKECHROOT_PATH_MAX-1 for both arguments.
| if ((linksize = nextcall(__readlink_chk)(path, tmp, FAKECHROOT_PATH_MAX-1, buflen)) == -1) { | ||
| if (__builtin_expect(!!(bufsiz > buflen), 0)) { | ||
| printf("readlink: prevented write past end of buffer\n"); | ||
| exit(-1); |
There was a problem hiding this comment.
why the exit? why not just return failure?
There was a problem hiding this comment.
Because _chk functions should exit on overflow. Actually __chk_fail should be called (similar to __realpath_chk).
https://refspecs.linuxbase.org/LSB_4.1.0/LSB-Core-generic/LSB-Core-generic/libc---readlink-chk-1.html
|
|
||
| if ((linksize = nextcall(__readlink_chk)(path, tmp, FAKECHROOT_PATH_MAX-1, buflen)) == -1) { | ||
| if (__builtin_expect(!!(bufsiz > buflen), 0)) { | ||
| printf("readlink: prevented write past end of buffer\n"); |
There was a problem hiding this comment.
why the printf to stdout? at most there is a print to stderr done in the rest of the codebase
| expand_chroot_path(path); | ||
|
|
||
| if ((linksize = nextcall(__readlink_chk)(path, tmp, FAKECHROOT_PATH_MAX-1, buflen)) == -1) { | ||
| if (__builtin_expect(!!(bufsiz > buflen), 0)) { |
There was a problem hiding this comment.
why the double exclamation mark? bufsiz > buflen is already a boolean
3 participants
Fixes #114