CVE-2015-2181 not fixed! #4958

Closed
rcubetrac opened this Issue Jan 20, 2016 · 16 comments

Comments

Projects
None yet
3 participants
@rcubetrac

Reported by malwared on 20 Jan 2016 17:17 UTC as Trac ticket #1490643

Hi guys, i really hope that was just mistake, misunderstanding... Issue still effects the product.

http://trac.roundcube.net/changeset/7c96646de0efda16cded8491138bfefe31aca940/github/plugins/password/helpers/chgdbmailusers.c

Keywords: buffer,overflow, badfix
Migrated-From: http://trac.roundcube.net/ticket/1490643

@rcubetrac

This comment has been minimized.

Show comment
Hide comment
@rcubetrac

rcubetrac Jan 21, 2016

Comment by @alecpl on 21 Jan 2016 07:30 UTC

The command length is checked in PHP, so there's no need to do this in c too. Or are you saing some more changes are needed. Please, provide a patch (or better a PR at github).

This is the full commit 7c96646

Comment by @alecpl on 21 Jan 2016 07:30 UTC

The command length is checked in PHP, so there's no need to do this in c too. Or are you saing some more changes are needed. Please, provide a patch (or better a PR at github).

This is the full commit 7c96646

@rcubetrac

This comment has been minimized.

Show comment
Hide comment
@rcubetrac

rcubetrac Jan 21, 2016

Severity changed by @alecpl on 21 Jan 2016 07:30 UTC

critical => major

Severity changed by @alecpl on 21 Jan 2016 07:30 UTC

critical => major

@rcubetrac

This comment has been minimized.

Show comment
Hide comment
@rcubetrac

rcubetrac Jan 21, 2016

Milestone changed by @alecpl on 21 Jan 2016 07:30 UTC

later => 1.1.5

Milestone changed by @alecpl on 21 Jan 2016 07:30 UTC

later => 1.1.5

@rcubetrac

This comment has been minimized.

Show comment
Hide comment
@rcubetrac

rcubetrac Jan 21, 2016

Comment by @alecpl on 21 Jan 2016 15:38 UTC

Anyway, 8ef598b should fix this.

Comment by @alecpl on 21 Jan 2016 15:38 UTC

Anyway, 8ef598b should fix this.

@rcubetrac

This comment has been minimized.

Show comment
Hide comment
@rcubetrac

rcubetrac Jan 21, 2016

Status changed by @alecpl on 21 Jan 2016 15:38 UTC

new => closed

Status changed by @alecpl on 21 Jan 2016 15:38 UTC

new => closed

@rcubetrac

This comment has been minimized.

Show comment
Hide comment
@rcubetrac

rcubetrac Jan 21, 2016

Comment by malwared on 21 Jan 2016 21:09 UTC

No and again NO! we faced to command execution vulnerability
rc = execvp(CMD, argv);

POC:

./chgdbmailusers ;id

Comment by malwared on 21 Jan 2016 21:09 UTC

No and again NO! we faced to command execution vulnerability
rc = execvp(CMD, argv);

POC:

./chgdbmailusers ;id
@rcubetrac

This comment has been minimized.

Show comment
Hide comment
@rcubetrac

rcubetrac Jan 21, 2016

Status changed by malwared on 21 Jan 2016 21:15 UTC

closed => reopened

Status changed by malwared on 21 Jan 2016 21:15 UTC

closed => reopened

@rcubetrac

This comment has been minimized.

Show comment
Hide comment
@rcubetrac

rcubetrac Jan 22, 2016

Comment by @alecpl on 22 Jan 2016 07:00 UTC

Other programs in password plugin use the same code. We're not C coders, could you provide a patch?

Comment by @alecpl on 22 Jan 2016 07:00 UTC

Other programs in password plugin use the same code. We're not C coders, could you provide a patch?

@rcubetrac

This comment has been minimized.

Show comment
Hide comment
@rcubetrac

rcubetrac Jan 22, 2016

Comment by @alecpl on 22 Jan 2016 16:09 UTC

And I don't get your POC. What it has to do with content of chgdbmailusers program? If you meant ./chgdbmailusers ";id" then it does not do what you think it would. And arguments are escaped in the driver (password/drivers/dbmail.php).

Comment by @alecpl on 22 Jan 2016 16:09 UTC

And I don't get your POC. What it has to do with content of chgdbmailusers program? If you meant ./chgdbmailusers ";id" then it does not do what you think it would. And arguments are escaped in the driver (password/drivers/dbmail.php).

@rcubetrac

This comment has been minimized.

Show comment
Hide comment
@rcubetrac

rcubetrac Jan 23, 2016

Comment by malwared on 23 Jan 2016 07:43 UTC

First of all we faced to command execution vulnerability and using it we have another privilege escalation vulnerability. Second i saw you are using escapeshellarg which previous time was bypassed by some researchers.

Comment by malwared on 23 Jan 2016 07:43 UTC

First of all we faced to command execution vulnerability and using it we have another privilege escalation vulnerability. Second i saw you are using escapeshellarg which previous time was bypassed by some researchers.

@rcubetrac

This comment has been minimized.

Show comment
Hide comment
@rcubetrac

rcubetrac Jan 25, 2016

Comment by @alecpl on 25 Jan 2016 09:41 UTC

Your comments aren't very helpful. The only issue I see is potentially vulnerable escapeshellarg(). I don't think we can implement a better method. The insecure input would be the password argument and the only solution I see is to allow only some set of (secure) ASCII characters. This does not sound right to me.

Comment by @alecpl on 25 Jan 2016 09:41 UTC

Your comments aren't very helpful. The only issue I see is potentially vulnerable escapeshellarg(). I don't think we can implement a better method. The insecure input would be the password argument and the only solution I see is to allow only some set of (secure) ASCII characters. This does not sound right to me.

@rcubetrac

This comment has been minimized.

Show comment
Hide comment
@rcubetrac

rcubetrac Feb 11, 2016

Comment by @alecpl on 11 Feb 2016 07:57 UTC

Thomas, what do you think? For me this is just "fixed".

Comment by @alecpl on 11 Feb 2016 07:57 UTC

Thomas, what do you think? For me this is just "fixed".

@rcubetrac

This comment has been minimized.

Show comment
Hide comment
@rcubetrac

rcubetrac Feb 11, 2016

Status changed by @alecpl on 11 Feb 2016 07:57 UTC

reopened => new

Status changed by @alecpl on 11 Feb 2016 07:57 UTC

reopened => new

@rcubetrac

This comment has been minimized.

Show comment
Hide comment
@rcubetrac

rcubetrac Feb 11, 2016

Owner changed by @alecpl on 11 Feb 2016 07:57 UTC

=> thomasb

Owner changed by @alecpl on 11 Feb 2016 07:57 UTC

=> thomasb

@rcubetrac

This comment has been minimized.

Show comment
Hide comment
@rcubetrac

rcubetrac Feb 12, 2016

Comment by malwared on 12 Feb 2016 20:35 UTC

Replying to alec:

Thomas, what do you think? For me this is just "fixed".

Dear Alec,
You need to add Validating argv to fix the issue, other wise we across with local privilege escalation vulnerability.

Comment by malwared on 12 Feb 2016 20:35 UTC

Replying to alec:

Thomas, what do you think? For me this is just "fixed".

Dear Alec,
You need to add Validating argv to fix the issue, other wise we across with local privilege escalation vulnerability.

@rcubetrac rcubetrac added this to the 1.1.5 milestone Mar 20, 2016

@alecpl alecpl removed the P3 label Apr 7, 2016

@thomascube

This comment has been minimized.

Show comment
Hide comment
@thomascube

thomascube Apr 17, 2016

Member

With the use of execvp added in 8ef598b, all arguments from argv are passed to the receiving dbmail-users binary. The execution of chgdbmailusers from PHP is composed with escaped arguments and results in something like ./chgdbmailusers -u 'foo' -w ';id'. I was not able to trick this into executing the id command as the above POC suggests.

Please re-open with a concrete exploit path and preferably with a patch.

Member

thomascube commented Apr 17, 2016

With the use of execvp added in 8ef598b, all arguments from argv are passed to the receiving dbmail-users binary. The execution of chgdbmailusers from PHP is composed with escaped arguments and results in something like ./chgdbmailusers -u 'foo' -w ';id'. I was not able to trick this into executing the id command as the above POC suggests.

Please re-open with a concrete exploit path and preferably with a patch.

@thomascube thomascube closed this Apr 17, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment