-
Notifications
You must be signed in to change notification settings - Fork 478
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
heap-buffer overflow in read_taskinfo() #938
Comments
If someone is interested, can contribute to this problem. |
Hello, i am trying to solve this issue So i tried implementing like following while (*endp != '\n') {
int tid = strtol(tids_str, &endp, 10);
+ if(nr_tid < info->nr_tid){
tids[nr_tid++] = tid;
+ }
+ else{
+ free(tids);
+ goto out;
+ }
- if (*endp != ',' && *endp != '\n') {
- free(tids);
- goto out;
- }
tids_str = endp + 1;
} how about it? |
Please take a look at the man page of |
Okay! thank you for your commant @namhyung . Can i understand your opinion more exactly? I understood this issue as following.
So my approach is to add exception handling that work well whatever value was written to And regarding did i understand this correctly? |
I think about the cases like the following Normal case of tid values
Abnormal case of tid values
So i tried implementing like below while (*endp != '\n') {
int tid = strtol(tids_str, &endp, 10);
if(nr_tid >= info->nr_tid){ // more tib values
free(tids);
goto out;
}
if(tid){ // normal case
tids[nr_tid++] = tid;
}
else { // include character except ',' or include Continuous `,`
free(tids);
goto out;
}
tids_str = endp + 1;
}
if(nr_tid < info->nr_tid){ // less tib values
free(tids);
goto out;
} |
You can also check if it's separated by |
Ah... i see
So the reflected code looks like below while (*endp != '\n') {
int tid = strtol(tids_str, &endp, 10);
if(nr_tid >= info->nr_tid){ // more tib values
free(tids);
goto out;
}
if(tid){ // normal case
tids[nr_tid++] = tid;
}
else { // include character except ',' or include Continuous `,`
free(tids);
goto out;
}
if (*endp != ',' && *endp != '\n') {
free(tids);
goto out;
}
tids_str = endp + 1;
}
if(nr_tid < info->nr_tid){ // less tib values
free(tids);
goto out;
} In my opinion, i think there are to many while (*endp != '\n') {
int tid = strtol(tids_str, &endp, 10);
if(tid && (nr_tid < info->nr_tid) && (*endp == ',' || *endp == '\n')){
tids[nr_tid++] = tid;
}
else {
free(tids);
goto out;
}
tids_str = endp + 1;
}
if(nr_tid < info->nr_tid){ // less tib values
free(tids);
goto out;
} |
@ParkSeungHyeok Please send a PR instead of dropping code snippets here. That's much easier for us to review the code. |
It'd be much better if you could add unittests for the cases you mentioned above. |
It fixes a heap-buffer overflow issue caused by data in taskinfo:tids. The root cause was insufficient exception handling for tid values. Added exception handling to manage any value written to tid Fixed: namhyung#938 Signed-off-by: Seunghyeok Park <tmdgur1324@naver.com>
It fixes a heap-buffer overflow issue caused by data in taskinfo:tids. The root cause was insufficient exception handling for tid values. Added exception handling to manage any value written to tids_str Fixed: namhyung#938 Signed-off-by: Seunghyeok Park <tmdgur1324@naver.com> Signed-off-by: ParkSeungHyeok <tmdgur1324@naver.com>
It fixes a heap-buffer overflow issue caused by data in taskinfo:tids. The root cause was insufficient exception handling for tid values. Added exception handling to manage any value written to tids_str Fixed: namhyung#938 Signed-off-by: Seunghyeok Park <tmdgur1324@naver.com>
It fixes a heap-buffer overflow issue caused by data in taskinfo:tids. The root cause was insufficient exception handling for tid values. Added exception handling to manage any value written to tids_str Fixed: namhyung#938 Signed-off-by: Seunghyeok Park <tmdgur1324@naver.com>
@namhyung @honggyukim |
Hello,
I found heap-buffer overflow bug.
If there is a large amount of data(
,
) in thetaskinfo:tids
of theinfo
file, it falls into an infiniteloop.
uftrace/cmds/info.c
Lines 560 to 569 in 2d6c907
PoC:
Crash info:
The text was updated successfully, but these errors were encountered: