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.

Sign up for GitHub

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

Jump to bottom

Radare2 reports incorrect function size #59

Open
dennisfischer opened this issue Sep 14, 2018 · 5 comments
Open

Radare2 reports incorrect function size #59

dennisfischer opened this issue Sep 14, 2018 · 5 comments
Labels
bug

Comments

@dennisfischer
Copy link
Collaborator

I'm observing an error during patching that is a result of an "incorrect" analysis of radare2.
The error occurs from the IR below.

; Function Attrs: noinline nounwind uwtable
define void @sig_handler(i32) #2 !input_dep_function !2 !sc_guard !3 {
  call void @registerFunction(i8* getelementptr inbounds ([12 x i8], [12 x i8]* @2, i32 0, i32 0))
  %2 = alloca i32, align 4, !input_indep_block !4, !input_dep_instr !5, !data_dep_instr !6, !control_dep_instr !7
  store i32 %0, i32* %2, align 4, !input_dep_instr !5, !data_dep_instr !6, !control_dep_instr !7, !argument_dep_instr !11
  call void @sig_handler0(), !input_indep_instr !9, !data_indep_instr !10, !control_dep_instr !7
  unreachable, !input_indep_instr !9, !data_indep_instr !10, !control_dep_instr !7
}

declare !sc_guard !3 i32 @puts(i8*) #3

; Function Attrs: noreturn nounwind
declare !sc_guard !3 void @exit(i32) #4

declare !sc_guard !3 i32 @system(i8*) #3

; Function Attrs: noinline nounwind uwtable
define void @alarm_handler(i32) #2 !input_dep_function !2 !sc_guard !3 {
  %local_hash1 = alloca i64, align 8
  store i64 0, i64* %local_hash1, align 8
  %local_hash = alloca i64, align 8
  store i64 0, i64* %local_hash, align 8
  call void @registerFunction(i8* getelementptr inbounds ([14 x i8], [14 x i8]* @3, i32 0, i32 0))
  %2 = alloca i32, align 4, !input_indep_block !4, !input_dep_instr !5, !data_dep_instr !6, !control_dep_instr !7
  store i32 %0, i32* %2, align 4, !input_dep_instr !5, !data_dep_instr !6, !control_dep_instr !7, !argument_dep_instr !11
  %3 = load i32, i32* %2, align 4, !input_dep_instr !5, !data_dep_instr !6, !control_dep_instr !7, !argument_dep_instr !11
  %4 = icmp ne i32 %3, 0, !input_dep_instr !5, !data_dep_instr !6, !control_dep_instr !7, !argument_dep_instr !11
  %5 = zext i1 %4 to i64
  call void @oh_hash2(i64* %local_hash1, i64 %5)
  br i1 %4, label %9, label %6, !input_dep_instr !5, !data_dep_instr !6, !control_dep_instr !7, !argument_dep_instr !11

; <label>:6:                                      ; preds = %1
  %7 = call i32 @sigsetup(i32 14, void (i32)* @alarm_handler), !data_indep_instr !10, !input_dep_block !12, !control_dep_instr !7, !argument_dep_instr !11
  call void @oh_hash2(i64* %local_hash1, i64 14)
  %8 = load i64, i64* %local_hash1
  store i64 %8, i64* @120
  call void @assert(i64* @120, i64 2000000000000), !oh_assert !13
  br label %9, !data_indep_instr !10, !control_dep_instr !7, !argument_dep_instr !11

; <label>:9:                                      ; preds = %6, %1
  store i64 0, i64* getelementptr inbounds (%struct.itimerval, %struct.itimerval* @alarm_handler.val, i32 0, i32 1, i32 0), align 8, !input_indep_block !4, !input_indep_instr !9, !data_indep_instr !10, !control_dep_instr !7
  call void @alarm_handler0(), !input_indep_instr !9, !data_indep_instr !10, !control_dep_instr !7
  call void @deregisterFunction(i8* getelementptr inbounds ([14 x i8], [14 x i8]* @4, i32 0, i32 0))
  %10 = load i64, i64* %local_hash
  store i64 %10, i64* @120
  call void @assert(i64* @120, i64 1000000000000), !oh_assert !13
  ret void, !input_indep_instr !9, !data_indep_instr !10, !control_dep_instr !7
}
define void @sig_handler0() !extracted !18 !input_dep_function !2 !sc_guard !3 {
entry:
  call void @registerFunction(i8* getelementptr inbounds ([13 x i8], [13 x i8]* @29, i32 0, i32 0))
  %0 = alloca %union.anon.8, align 4, !input_indep_block !4, !input_dep_instr !5, !data_dep_instr !6, !control_dep_instr !7
  %1 = call i32 @puts(i8* getelementptr inbounds ([11 x i8], [11 x i8]* @.str, i32 0, i32 0)), !input_dep_instr !5, !data_dep_instr !6, !control_dep_instr !7
  %2 = bitcast %union.anon.8* %0 to i32*, !input_dep_instr !5, !data_dep_instr !6, !control_dep_instr !7
  %3 = call i32 @system(i8* getelementptr inbounds ([10 x i8], [10 x i8]* @.str.1, i32 0, i32 0)), !input_dep_instr !5, !data_dep_instr !6, !control_dep_instr !7
  store i32 %3, i32* %2, align 4, !input_dep_instr !5, !data_dep_instr !6, !control_dep_instr !7
  %4 = bitcast %union.anon.8* %0 to i32*, !input_dep_instr !5, !data_dep_instr !6, !control_dep_instr !7
  %5 = load i32, i32* %4, align 4, !input_dep_instr !5, !data_dep_instr !6, !control_dep_instr !7
  %6 = and i32 %5, 65280, !input_dep_instr !5, !data_dep_instr !6, !control_dep_instr !7
  %7 = ashr i32 %6, 8, !input_dep_instr !5, !data_dep_instr !6, !control_dep_instr !7
  call void @exit(i32 %7) #9, !input_dep_instr !5, !data_dep_instr !6, !control_dep_instr !7
  call void @deregisterFunction(i8* getelementptr inbounds ([13 x i8], [13 x i8]* @30, i32 0, i32 0))
  ret void, !input_indep_instr !9, !data_indep_instr !10, !control_dep_instr !7
}

objdump

0000000000401820 <sig_handler>:
  401820:	55                   	push   %rbp
  401821:	48 89 e5             	mov    %rsp,%rbp
  401824:	53                   	push   %rbx
  401825:	50                   	push   %rax
  401826:	89 fb                	mov    %edi,%ebx
  401828:	bf 4a 47 40 00       	mov    $0x40474a,%edi
  40182d:	e8 ce fd ff ff       	callq  401600 <registerFunction@plt>
  401832:	89 5d f4             	mov    %ebx,-0xc(%rbp)
  401835:	e8 26 13 00 00       	callq  402b60 <sig_handler0>
  40183a:	66 0f 1f 44 00 00    	nopw   0x0(%rax,%rax,1)

0000000000401840 <alarm_handler>:
  401840:	55                   	push   %rbp
  401841:	48 89 e5             	mov    %rsp,%rbp
  401844:	53                   	push   %rbx
  401845:	48 83 ec 18          	sub    $0x18,%rsp
  401849:	89 fb                	mov    %edi,%ebx
  40184b:	48 c7 45 f0 00 00 00 	movq   $0x0,-0x10(%rbp)
  401852:	00 
  401853:	48 c7 45 e8 00 00 00 	movq   $0x0,-0x18(%rbp)
  40185a:	00 
  40185b:	bf 56 47 40 00       	mov    $0x404756,%edi
  401860:	e8 9b fd ff ff       	callq  401600 <registerFunction@plt>
  401865:	89 5d e4             	mov    %ebx,-0x1c(%rbp)
  401868:	31 f6                	xor    %esi,%esi
  40186a:	85 db                	test   %ebx,%ebx
  40186c:	40 0f 95 c6          	setne  %sil
  401870:	48 8d 7d f0          	lea    -0x10(%rbp),%rdi
  401874:	e8 e7 fd ff ff       	callq  401660 <oh_hash2@plt>
  401879:	85 db                	test   %ebx,%ebx
  40187b:	75 3c                	jne    4018b9 <alarm_handler+0x79>
  40187d:	bf 0e 00 00 00       	mov    $0xe,%edi
  401882:	be 40 18 40 00       	mov    $0x401840,%esi
  401887:	e8 d4 fe ff ff       	callq  401760 <sigsetup>
  40188c:	48 8d 7d f0          	lea    -0x10(%rbp),%rdi
  401890:	be 0e 00 00 00       	mov    $0xe,%esi
  401895:	e8 c6 fd ff ff       	callq  401660 <oh_hash2@plt>
  40189a:	48 8b 45 f0          	mov    -0x10(%rbp),%rax
  40189e:	48 89 05 1b 48 20 00 	mov    %rax,0x20481b(%rip)        # 6060c0 <__unnamed_5>
  4018a5:	48 be 00 20 4a a9 d1 	movabs $0x1d1a94a2000,%rsi
  4018ac:	01 00 00 
  4018af:	bf c0 60 60 00       	mov    $0x6060c0,%edi
  4018b4:	e8 17 fd ff ff       	callq  4015d0 <assert@plt>
  4018b9:	48 c7 05 14 48 20 00 	movq   $0x0,0x204814(%rip)        # 6060d8 <alarm_handler.val+0x10>
  4018c0:	00 00 00 00 
  4018c4:	e8 d7 12 00 00       	callq  402ba0 <alarm_handler0>
  4018c9:	bf 56 47 40 00       	mov    $0x404756,%edi
  4018ce:	e8 4d fd ff ff       	callq  401620 <deregisterFunction@plt>
  4018d3:	48 8b 45 e8          	mov    -0x18(%rbp),%rax
  4018d7:	48 89 05 e2 47 20 00 	mov    %rax,0x2047e2(%rip)        # 6060c0 <__unnamed_5>
  4018de:	48 be 00 10 a5 d4 e8 	movabs $0xe8d4a51000,%rsi
  4018e5:	00 00 00 
  4018e8:	bf c0 60 60 00       	mov    $0x6060c0,%edi
  4018ed:	e8 de fc ff ff       	callq  4015d0 <assert@plt>
  4018f2:	48 83 c4 18          	add    $0x18,%rsp
  4018f6:	5b                   	pop    %rbx
  4018f7:	5d                   	pop    %rbp
  4018f8:	c3                   	retq   
  4018f9:	0f 1f 80 00 00 00 00 	nopl   0x0(%rax)

radare2 aa + afl

[0x00401670]> afl
0x00400000    5 314  -> 249  sym.imp.__libc_start_main
0x00401540    3 23           sym._init
0x00401570    1 6            sym.imp.printf
0x00401580    1 6            sym.imp.puts
0x00401590    1 6            sym.imp.exit
0x004015a0    1 6            sym.imp.oh_hash1
0x004015b0    1 6            sym.imp.setitimer
0x004015c0    1 6            sym.imp.system
0x004015d0    1 6            sym.imp.assert
0x004015e0    1 6            sym.imp.guardMe
0x004015f0    1 6            sym.imp.sigaction
0x00401600    1 6            sym.imp.registerFunction
0x00401610    1 6            sym.imp.getchar
0x00401620    1 6            sym.imp.deregisterFunction
0x00401630    1 6            sym.imp.verifyStack
0x00401640    1 6            sym.imp.sigemptyset
0x00401650    1 6            sym.imp.rand
0x00401660    1 6            sym.imp.oh_hash2
0x00401670    1 43           entry0
0x004016a0    1 2            sym._dl_relocate_static_pie
0x004016b0    3 35           sym.deregister_tm_clones
0x004016e0    3 53           sym.register_tm_clones
0x00401720    3 34   -> 29   sym.__do_global_dtors_aux
0x00401750    1 7            entry1.init
0x00401760    4 182          sym.sigsetup
0x00401820    3 217          sym.sig_handler
0x00401900    1 49           sym.show_score
0x00401940    8 152  -> 137  sym.draw_line
0x004019e0   34 682  -> 644  sym.setup_level
0x00401c90   23 562  -> 411  sym.move
0x00401ed0    7 116          sym.collide_walls
0x00401f50    4 1038         sym.collide_object
0x00402360    8 173  -> 161  sym.collide_self
0x00402410    4 109          sym.collision
0x00402480    3 156          sym.eat_gold
0x00402520   17 417  -> 398  main
0x004026d0    1 574          sym.sigsetup0
0x00402910    1 579          sym.sigsetup1
0x00402b60    1 42           sym.sig_handler0
0x00402ba0    1 50           sym.alarm_handler0
0x00402be0    1 309          sym.show_score0
0x00402d20    1 140          sym.draw_line0
0x00402db0    1 151          sym.setup_level0
0x00402e50    1 60           sym.setup_level1
0x00402e90    1 217          sym.setup_level2
0x00402f70    1 129          sym.setup_level3
0x00403000    1 171          sym.setup_level4
0x004030b0    1 231          sym.setup_level5
0x004031a0    1 155          sym.setup_level6
0x00403240    1 57           sym.setup_level7
0x00403280    1 101          sym.setup_level8
0x004032f0    1 168          sym.setup_level9
0x004033a0    1 184          sym.setup_level10
0x00403460    1 98           sym.setup_level11
0x004034d0    1 107          sym.setup_level12
0x00403540    1 74           sym.move0
0x00403590    1 69           sym.move1
0x004035e0    1 69           sym.move2
0x00403630    1 74           sym.move3
0x00403680    1 74           sym.move4
0x004036d0    1 69           sym.move5
0x00403720    1 69           sym.move6
0x00403770    1 113          sym.move7
0x004037f0    1 69           sym.move8
0x00403840    1 113          sym.move9
0x004038c0    1 69           sym.move10
0x00403910    1 113          sym.move11
0x00403990    1 88           sym.move12
0x004039f0    1 132          sym.move13
0x00403a80    1 88           sym.move14
0x00403ae0    1 88           sym.move15
0x00403b40    1 152          sym.move16
0x00403be0    1 105          sym.move17
0x00403c50    1 101          sym.move18
0x00403cc0    1 129          sym.move19
0x00403d50    1 57           sym.move20
0x00403d90    1 107          sym.collide_self0
0x00403e00    1 57           sym.collide_self1
0x00403e40    1 73           sym.eat_gold0
0x00403e90    1 41           sym.main0
0x00403ec0    1 135          sym.main1
0x00403f50    1 66           sym.main2
0x00403fa0    1 97           sym.main3
0x00404010    1 405          sym.collide_object_path_0
0x004041b0    1 377          sym.collide_object_path_1
0x00404330    1 402          sym.collide_object_path_2
0x004044d0    1 79           sym.alarm_handler_path_0
0x00404520    1 95           sym.alarm_handler_path_1
0x00404580    1 28           sym.oh_path_calls
0x004045a0    1 8            sym.oh_path_functions
0x004045b0    4 101          sym.__libc_csu_init
0x00404620    1 2            sym.__libc_csu_fini
0x00404624    1 9            sym._fini

Observation:
Radare2 identifies sig_handler as size 217 (0xd9). However, this is too large and includes alarm_handler function. The patcher receives this size and computes a hash that spans both functions. If alarm_handler contains any other guard that needs to be patched before sig_handler, then this can result in an incorrect hash at runtime.

Further, sig_handler contains unreachable, but no ret, thus also no ret instruction is present in the binary (possible reason why radare2 fails!).

However, have a look at the function before extraction was applied. It contains ret void that is now missing in sig_handler and is instead only in sig_handler0.

; Function Attrs: nounwind uwtable
define void @sig_handler(i32) #0 {
  %2 = alloca i32, align 4
  %3 = alloca %union.anon.8, align 4
  store i32 %0, i32* %2, align 4
  %4 = call i32 @puts(i8* getelementptr inbounds ([11 x i8], [11 x i8]* @.str, i32 0, i32 0))
  %5 = bitcast %union.anon.8* %3 to i32*
  %6 = call i32 @system(i8* getelementptr inbounds ([10 x i8], [10 x i8]* @.str.1, i32 0, i32 0))
  store i32 %6, i32* %5, align 4
  %7 = bitcast %union.anon.8* %3 to i32*
  %8 = load i32, i32* %7, align 4
  %9 = and i32 %8, 65280
  %10 = ashr i32 %9, 8
  call void @exit(i32 %10) #6
  unreachable
                                                  ; No predecessors!
  ret void
}
@dennisfischer dennisfischer mentioned this issue Sep 14, 2018
Closed
@dennisfischer
Copy link
Collaborator Author

I've checked with radare2 that the missing ret is the problem. Inserting a ret between both functions gives a correct analysis.

@mr-ma
Copy link
Collaborator

mr-ma commented Sep 14, 2018

Extraction pass is part of the input dependency analyser @anahitH

@dennisfischer
Copy link
Collaborator Author

Reason is the wrong analysis of radare2.

@dennisfischer
Copy link
Collaborator Author

So far there seems to be no way to reliably find the size of a function. In my results I observed that the size seems to be incorrect only for the function main. According to r2 maintainers, the size of the function that is reported by the analysis is allowed to be wrong. They wont consider this as a bug.
I tried to use the symbols instead which are reported by fj symbols r2 command. This reduces the amount of errors significantly because the size of the main function is closer to the realsize. However, I'm still able to find cases in which the size is incorrect.

Currently it looks like the only way to solve this problem is to not use main as a Checkee.

@mr-ma
Copy link
Collaborator

mr-ma commented Sep 16, 2018

Alright, let's keep this issue open until we find an adequate solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@dennisfischer @mr-ma