Skip to content
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

Use r_str_scanf as a safe alternative to fix gdb reg profile parsing bugs #22562

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
7bc0349
Fix conversion from GDB register profile to rizin profile
Crispy-fried-chicken Feb 3, 2024
666c94f
delete some defination as for the reviewer
Crispy-fried-chicken Feb 4, 2024
e9b636b
update for some foramt
Crispy-fried-chicken Feb 4, 2024
d9854e2
fix some bugs for sscanf and update for reviewer's suggestions
Crispy-fried-chicken Feb 6, 2024
8e8a040
fix redeclaration
Crispy-fried-chicken Feb 6, 2024
8ca150d
fix
Crispy-fried-chicken Feb 6, 2024
197a3cc
update for some fix
Crispy-fried-chicken Feb 6, 2024
fa32abb
Merge branch 'master' into patch-2
Crispy-fried-chicken Feb 7, 2024
c3c3ee3
Merge branch 'master' into patch-2
Crispy-fried-chicken Feb 8, 2024
292fa8c
change sscanf to r_str_scanf to fix OOB
Crispy-fried-chicken Feb 8, 2024
b25fd13
fix
Crispy-fried-chicken Feb 8, 2024
ecab0cc
fix
Crispy-fried-chicken Feb 8, 2024
5aa813f
add some check for return value
Crispy-fried-chicken Feb 14, 2024
3003235
Merge branch 'master' into patch-2
Crispy-fried-chicken Feb 14, 2024
e76a620
Apply suggestions from code review
trufae Feb 14, 2024
14c6da7
Apply suggestions from code review
trufae Feb 14, 2024
1853264
fix some lint error
Crispy-fried-chicken Feb 14, 2024
8adfa77
Merge branch 'patch-2' of https://github.com/Crispy-fried-chicken/rad…
Crispy-fried-chicken Feb 14, 2024
a748b70
Merge branch 'master' into patch-2
Crispy-fried-chicken Feb 14, 2024
1cc22fe
Update libr/io/p/io_self.c
trufae Feb 14, 2024
a06f017
Update libr/debug/p/debug_gdb.c
trufae Feb 14, 2024
0e34ceb
Update libr/debug/p/debug_gdb.c
trufae Feb 14, 2024
8793f0f
fix
Crispy-fried-chicken Feb 14, 2024
4cc4769
Apply suggestions from code review
trufae Feb 14, 2024
6dcbd85
Update libr/debug/p/native/linux/linux_coredump.c
trufae Feb 14, 2024
97a28ad
Update libr/debug/p/native/linux/linux_coredump.c
trufae Feb 14, 2024
dbeadf1
Update libr/debug/p/debug_native.c
trufae Feb 14, 2024
c193c46
Update libr/debug/p/debug_native.c
trufae Feb 14, 2024
c3c0906
Apply suggestions from code review
trufae Feb 14, 2024
8698e5a
Update libr/debug/p/native/linux/linux_coredump.c
trufae Feb 14, 2024
d4f2fba
Apply suggestions from code review
trufae Feb 14, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions libr/debug/p/debug_gdb.c
Original file line number Diff line number Diff line change
Expand Up @@ -236,12 +236,8 @@ static RList *r_debug_gdb_map_get(RDebug* dbg) { // TODO
r_list_free (words);
free (wordstr);
#else
// We assume Linux target, for now, so -
// 7ffff7dda000-7ffff7dfd000 r-xp 00000000 08:05 265428 /usr/lib/ld-2.25.so
// Android
// "12c00000-12c40000 rw-p 00000000 00:00 0 [anon:dalvik-main space (region space)]";
ret = sscanf (ptr, "%s %s %"PFMT64x" %*s %*s %[^\n]", &region1[2],
perms, &offset, name);
ret = r_str_scanf (ptr, "%.s %.s %Lx %*s %*s %.[^\n]",
sizeof (region1) - 2, region1 + 2, sizeof (perms), perms, offset, sizeof (name), name);
#endif
// eprintf ("RET = %d (%s)\n", ret, ptr);
if (ret == 3) {
Expand Down
5 changes: 3 additions & 2 deletions libr/debug/p/debug_io.c
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,9 @@ static RList *__io_maps(RDebug *dbg) {
if (_s_) {
memmove (_s_, _s_ + 2, strlen (_s_));
}
sscanf (str, "0x%"PFMT64x" - 0x%"PFMT64x" %s %s",
&map_start, &map_end, perm, name);
if (r_str_scanf (str, "0x%Lx - 0x%Lx %.s %.s", &map_start, &map_end, sizeof (perm), perm, sizeof (name), name) > 4) {
break;
}
if (map_end != 0LL) {
RDebugMap *map = r_debug_map_new (name, map_start, map_end, r_str_rwx (perm), 0);
r_list_append (list, map);
Expand Down
7 changes: 3 additions & 4 deletions libr/debug/p/debug_native.c
Original file line number Diff line number Diff line change
Expand Up @@ -1091,9 +1091,8 @@ static RList *r_debug_native_map_get(RDebug *dbg) {
}
#if __KFBSD__
// 0x8070000 0x8072000 2 0 0xc1fde948 rw- 1 0 0x2180 COW NC vnode /usr/bin/gcc
if (sscanf (line, "%s %s %d %d 0x%s %3s %d %d",
&region[2], &region2[2], &ign, &ign,
unkstr, perms, &ign, &ign) != 8) {
if (r_str_scanf (line, "%.s %.s %*d %*d %*s %.s %*d %*d",
sizeof (region)-2, &region[2], sizeof (region2)-2, &region2[2], sizeof (perms), perms) != 3) {
R_LOG_ERROR ("%s: Unable to parse \"%s\"", __func__, path);
r_list_free (list);
return NULL;
Expand All @@ -1109,7 +1108,7 @@ static RList *r_debug_native_map_get(RDebug *dbg) {
#else
ut64 offset = 0;
// 7fc8124c4000-7fc81278d000 r--p 00000000 fc:00 17043921 /usr/lib/locale/locale-archive
i = sscanf (line, "%s %s %08"PFMT64x" %*s %*s %[^\n]", &region[2], perms, &offset, name);
i = r_str_scanf (line, "%.s %.s %Lx %*s %*s %.[^\n]", sizeof (region[2]), &region[2], sizeof (perms), perms, &offset, sizeof (name), name);
if (i == 3) {
name[0] = '\0';
} else if (i != 4) {
Expand Down
28 changes: 14 additions & 14 deletions libr/debug/p/native/linux/linux_coredump.c
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* radare - LGPL - Copyright 2016-2023 - Oscar Salvador */

#include <r_debug.h>
#include <r_util.h>

#if DEBUGGER

Expand Down Expand Up @@ -157,9 +158,7 @@ static proc_per_thread_t *get_proc_thread_content(int pid, int tid) {
int no_num;
char no_char;
ut32 no_ui;
sscanf (buff, "%d %s %c %d %d %d %d %d %u %lu %lu %lu %lu"
"%"PFMT64x" %"PFMT64x" %ld %lu",
&no_num, no_str, &no_char, &no_num, &no_num, &no_num,
r_str_scanf (buff, "%d %.s %c %d %d %d %d %d %u %lu %lu %lu %Lx %Lx %ld %lu", &no_num, sizeof (no_str), no_str, &no_char, &no_num, &no_num, &no_num,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can get rid of all those no* variables if we use %*d in the arguments instead

&no_num, &no_num, &no_ui, &no_lui, &no_lui, &no_lui,
&no_lui, &t->utime, &t->stime, &t->cutime, &t->cstime);
free (buff);
Expand Down Expand Up @@ -809,16 +808,13 @@ static proc_per_process_t *get_proc_process_content(RDebug *dbg) {
/* /proc/[pid]/stat */
/* we only need few fields which are process-wide */
{
char no_str[128];
long unsigned int no_lui;
long int no_li;
int no_num;
sscanf (buff, "%d %s %c %d %d %d %d %d %u %lu %lu %lu %lu"
"%lu %lu %ld %ld %ld %ld %ld",
&p->pid, no_str, &p->s_name, &p->ppid, &p->pgrp, &no_num,
&no_num, &p->sid, &p->flag, &no_lui, &no_lui, &no_lui,
&no_lui, &no_lui, &no_lui, &no_li, &no_li,
&no_li, &p->nice, &p->num_threads);
if (r_str_scanf (buff, "%d %*s %c %d %d %*d %*d %lu %ld",
&p->pid, &p->s_name, &p->ppid, &p->pgrp,
&p->sid, &p->flag,
&p->nice, &p->num_threads) < 6) {
free (buff);
return NULL;
}
free (buff);
}
if (!p->num_threads || p->num_threads < 1) {
Expand Down Expand Up @@ -869,7 +865,11 @@ static proc_per_process_t *get_proc_process_content(RDebug *dbg) {
file = r_strf ("/proc/%d/coredump_filter", dbg->pid);
buff = r_file_slurp (file, &size);
if (buff) {
sscanf (buff, "%hx", &filter_flags);
if (r_str_scanf (buff, "%hx", &filter_flags) != 1) {
free (p);
Crispy-fried-chicken marked this conversation as resolved.
Show resolved Hide resolved
free (buff);
return NULL;
}
p->coredump_filter = filter_flags;
free (buff);
} else {
Expand Down
1 change: 1 addition & 0 deletions libr/include/r_types_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -293,4 +293,5 @@ static inline void *untagged_pointer_check(void *x) {
}
#endif


#endif // R2_TYPES_BASE_H
4 changes: 3 additions & 1 deletion libr/io/p/io_self.c
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,9 @@ static int update_self_regions(RIO *io, int pid) {
}
path[0]='\0';
strcpy (region, "0x");
sscanf (line, "%s %s %*s %*s %*s %[^\n]", region + 2, perms, path);
if (r_str_scanf (line, "%.s %.s %*s %*s %*s %.[^\n]", sizeof (region) - 2, region + 2, sizeof (perms), perms, sizeof (path), path) < 6) {
return false;
}
pos_c = strchr (region + 2, '-');
if (pos_c) {
*pos_c++ = 0;
Expand Down
5 changes: 2 additions & 3 deletions libr/reg/profile.c
Original file line number Diff line number Diff line change
Expand Up @@ -350,9 +350,8 @@ static char *gdb_to_r2_profile(const char *gdb) {
r_strbuf_free (sb);
return false;
}
ret = sscanf (ptr, " %s %d %d %d %d %s %s", name, &number, &rel,
&offset, &size, type, groups);
// Groups is optional, others not
ret = r_str_scanf (ptr, "%.s %d %d %d %d %.s %.s", sizeof (name), name, &number, &rel, &offset, &size, sizeof (type), type, sizeof (groups), groups);
// Groups is optional, others are not
if (ret < 6) {
if (*ptr != '*') {
R_LOG_WARN ("Could not parse line: %s", ptr);
Expand Down
Loading