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

A potential Integer Overflow bug found in aplay/aplay.c #231

Open
x14ngch3n opened this issue Aug 20, 2023 · 0 comments
Open

A potential Integer Overflow bug found in aplay/aplay.c #231

x14ngch3n opened this issue Aug 20, 2023 · 0 comments

Comments

@x14ngch3n
Copy link

x14ngch3n commented Aug 20, 2023

Hi, I'm currently trying to use the static analysis tool Infer to find uncatched API-misuse bugs in OpenWrt packages, and I find a potential Integer Overflow in your project, version 1.2.9.

The bug located in aplay/aplay.c. Firstly, the program tries to write the remains bytes in audiobuf with the length of loaded in line 2865, and loaded is later used as the parameter for safe_read in the loop, then loaded is used as the 2nd argument of pct_write() and finally after a multiply operation, it is used as the size of Malloc in remap_data(), as shown in the following code:

static void playback_go(int fd, size_t loaded, off_t count, int rtype, char *name)
{
int l, r;
off_t written = 0;
off_t c;

header(rtype, name);
set_params();

while (loaded > chunk_bytes && written < count && !in_aborting) {
	if (pcm_write(audiobuf + written, chunk_size) <= 0)
		return;
	written += chunk_bytes;
	loaded -= chunk_bytes;
}
if (written > 0 && loaded > 0)
	memmove(audiobuf, audiobuf + written, loaded);

l = loaded;
while (written < count && !in_aborting) {
	do {
		c = count - written;
		if (c > chunk_bytes)
			c = chunk_bytes;

		/* c < l, there is more data loaded
			* then we actually need to write
			*/
		if (c < l)
			l = c;

		c -= l;

		if (c == 0)
			break;
		r = safe_read(fd, audiobuf + l, c);
		if (r < 0) {
			perror(name);
			prg_exit(EXIT_FAILURE);
		}
		fdcount += r;
		if (r == 0)
			break;
		l += r;
	} while ((size_t)l < chunk_bytes);
	l = l * 8 / bits_per_frame;
	r = pcm_write(audiobuf, l);
	if (r != l)
		break;
	r = r * bits_per_frame / 8;
	written += r;
	l = 0;
}
if (!in_aborting) {
	snd_pcm_nonblock(handle, 0);
	snd_pcm_drain(handle);
	snd_pcm_nonblock(handle, nonblock);
}
}

# in remap_data()
chunk_bytes = count * bits_per_frame / 8;
if (tmp_size < chunk_bytes) {
	free(tmp);
	tmp = malloc(chunk_bytes);
	if (!tmp) {
		error(_("not enough memory"));
		exit(1);
	}
	tmp_size = count;
}

The parameter passed to Malloc may be overflowed so that the actual allocated memory is small.

I also attached the analysis trace given by Infer FYI:

 "trace": [
{
	"file": "aplay/aplay.c",
	"line": 940,
	"col": 14,
	"feature": [ "Input", "read" ]
},
{
	"file": "aplay/aplay.c",
	"line": 940,
	"col": 14,
	"feature": [ "Input", "read" ]
},
{
	"file": "aplay/aplay.c",
	"line": 2865,
	"col": 21,
	"feature": [
	"Prune",
	[
		"UnOp",
		"!",
		[
		"BinOp",
		">",
		[ "Var" ],
		[ "Cast", [ "Unsupported" ], [ "Const", [ "Cint", 0 ] ] ]
		]
	]
	]
},
{
	"file": "aplay/aplay.c",
	"line": 2868,
	"col": 2,
	"feature": [ "Store", [ "Var" ], [ "Var" ] ]
},
{
	"file": "aplay/aplay.c",
	"line": 2878,
	"col": 8,
	"feature": [
	"Prune",
	[ "UnOp", "!", [ "BinOp", "<", [ "Var" ], [ "Var" ] ] ]
	]
},
{
	"file": "aplay/aplay.c",
	"line": 2871,
	"col": 4,
	"feature": [
	"Store",
	[ "Var" ],
	[ "BinOp", "-", [ "Var" ], [ "Var" ] ]
	]
},
{
	"file": "aplay/aplay.c",
	"line": 2872,
	"col": 8,
	"feature": [
	"Prune",
	[
		"UnOp",
		"!",
		[
		"BinOp",
		">",
		[ "Cast", [ "Unsupported" ], [ "Var" ] ],
		[ "Var" ]
		]
	]
	]
},
{
	"file": "aplay/aplay.c",
	"line": 2878,
	"col": 8,
	"feature": [ "Prune", [ "BinOp", "<", [ "Var" ], [ "Var" ] ] ]
},
{
	"file": "aplay/aplay.c",
	"line": 2879,
	"col": 5,
	"feature": [ "Store", [ "Var" ], [ "Var" ] ]
},
{
	"file": "aplay/aplay.c",
	"line": 2899,
	"col": 3,
	"feature": [
	"Store",
	[ "Var" ],
	[
		"BinOp",
		"/",
		[
		"BinOp",
		"*",
		[ "Cast", [ "Unsupported" ], [ "Var" ] ],
		[ "Var" ]
		],
		[ "Cast", [ "Unsupported" ], [ "Const", [ "Cint", 8 ] ] ]
	]
	]
},
{
	"file": "aplay/aplay.c",
	"line": 2900,
	"col": 3,
	"feature": [
	"Store",
	[ "Var" ],
	[ "BinOp", "+", [ "Var" ], [ "Var" ] ]
	]
},
{
	"file": "aplay/aplay.c",
	"line": 2871,
	"col": 4,
	"feature": [
	"Store",
	[ "Var" ],
	[ "BinOp", "-", [ "Var" ], [ "Var" ] ]
	]
},
{
	"file": "aplay/aplay.c",
	"line": 2872,
	"col": 8,
	"feature": [
	"Prune",
	[
		"UnOp",
		"!",
		[
		"BinOp",
		">",
		[ "Cast", [ "Unsupported" ], [ "Var" ] ],
		[ "Var" ]
		]
	]
	]
},
{
	"file": "aplay/aplay.c",
	"line": 2878,
	"col": 8,
	"feature": [ "Prune", [ "BinOp", "<", [ "Var" ], [ "Var" ] ] ]
},
{
	"file": "aplay/aplay.c",
	"line": 2879,
	"col": 5,
	"feature": [ "Store", [ "Var" ], [ "Var" ] ]
},
{
	"file": "aplay/aplay.c",
	"line": 2895,
	"col": 3,
	"feature": [
	"Store",
	[ "Var" ],
	[
		"BinOp",
		"/",
		[
		"Cast",
		[ "Unsupported" ],
		[ "BinOp", "*", [ "Var" ], [ "Const", [ "Cint", 8 ] ] ]
		],
		[ "Var" ]
	]
	]
},
{
	"file": "aplay/aplay.c",
	"line": 2896,
	"col": 7,
	"feature": [ "Call", "pcm_write" ]
},
{
	"file": "aplay/aplay.c",
	"line": 2126,
	"col": 6,
	"feature": [
	"Prune",
	[ "UnOp", "!", [ "BinOp", "<", [ "Var" ], [ "Var" ] ] ]
	]
},
{
	"file": "aplay/aplay.c",
	"line": 2130,
	"col": 9,
	"feature": [ "Call", "remap_data" ]
},
{
	"file": "aplay/aplay.c",
	"line": 2069,
	"col": 2,
	"feature": [
	"Store",
	[ "Var" ],
	[
		"BinOp",
		"/",
		[ "BinOp", "*", [ "Var" ], [ "Var" ] ],
		[ "Cast", [ "Unsupported" ], [ "Const", [ "Cint", 8 ] ] ]
	]
	]
},
{
	"file": "aplay/aplay.c",
	"line": 2070,
	"col": 6,
	"feature": [ "Prune", [ "BinOp", "<", [ "Var" ], [ "Var" ] ] ]
},
{
	"file": "aplay/aplay.c",
	"line": 2072,
	"col": 9,
	"feature": [ "IntOverflow", "malloc", [ "Var" ] ]
}
],
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant