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

Lack of Validation Check for r_asm_pseudo_incbin at libr/asm/asm.c #15545

Closed
mmmdzz opened this issue Dec 6, 2019 · 1 comment
Closed

Lack of Validation Check for r_asm_pseudo_incbin at libr/asm/asm.c #15545

mmmdzz opened this issue Dec 6, 2019 · 1 comment
Labels

Comments

@mmmdzz
Copy link

mmmdzz commented Dec 6, 2019

Work environment

Questions Answers
OS/arch/bits (mandatory) Ubuntu x86 64
File format of the file you reverse (mandatory) None
Architecture/bits of the file (mandatory) None
r2 -v full output, not truncated (mandatory) radare2 4.1.0-git 23530 @ linux-x86-64 git.4.0.0-174-gb7cc6999a commit: b7cc699 build: 2019-12-05__21:51:53

Expected behavior

$ r2 malloc://1024
[0x00000000]> /a .incbin NoExistFile 0 0  # Expect No Crash

Actual behavior

$ r2 malloc://1024
[0x00000000]> /a .incbin NoExistFile 0 0 
Segmentation fault

Steps to reproduce the behavior

  • Please follow the steps I list above

Additional Logs, screenshots, source-code, configuration dump, ...

At libr/asm/asm.c, the lack of validation check of variable content will cause crash and arbitrary read via craft input.

below is the vulnerable code.

static inline int r_asm_pseudo_incbin(RAsmOp *op, char *input) {
	int bytes_read = 0;
	r_str_replace_char (input, ',', ' ');
	// int len = r_str_word_count (input);
	r_str_word_set0 (input);
	//const char *filename = r_str_word_get0 (input, 0);
	int skip = (int)r_num_math (NULL, r_str_word_get0 (input, 1));
	int count = (int)r_num_math (NULL,r_str_word_get0 (input, 2));
	char *content = r_file_slurp (input, &bytes_read);
	if (skip > 0) {
		skip = skip > bytes_read ? bytes_read : skip;
	}
	if (count > 0) {
		count = count > bytes_read ? 0 : count;
	} else {
		count = bytes_read;
	}
	// Need to handle arbitrary amount of data
	r_buf_free (op->buf_inc);
	op->buf_inc = r_buf_new_with_string (content + skip);
	// Terminate the original buffer
	free (content);
	return count;
}

If r_file_slurp tries to open an invalid file, content will be NULL. Later, because skip is the input number, r_buf_new_with_string (content + skip) will cause crash, or arbitrary write via crafted input.

@carnil
Copy link

carnil commented Dec 13, 2019

Seems that CVE-2019-19647 is assigned for this issue.

@radare radare added this to the 4.1.0 (Antull) milestone Dec 13, 2019
@radare radare added the crash label Dec 13, 2019
GustavoLCR added a commit to GustavoLCR/radare2 that referenced this issue Dec 15, 2019
@radare radare closed this as completed in 07b5e06 Dec 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants