A security audit of the Fetchmail plugin for Roundcube webmail has uncovered multiple SQL injection vulnerabilities and encryption weaknesses. These vulnerabilities could allow an authenticated attacker to execute arbitrary SQL commands against the database, potentially leading to unauthorized data access, data modification, or in some cases, server compromise. Additionally, the password storage mechanism used base64 encoding rather than strong encryption, exposing sensitive credentials to potential compromise.
Roundcube Fetchmail Plugin (fetchmail.php
and fetchmail.pl
)
High
The original Fetchmail plugin code directly embedded user input into SQL queries using string concatenation rather than using parameterized queries. This practice is vulnerable to SQL injection attacks where an attacker could manipulate input to modify the intended SQL query structure. Additionally, password storage relied on base64 encoding (which is easily reversible) rather than strong encryption, and there were logical errors in conditional statements that could lead to unexpected behavior.
In the original code:
if ($id != 0 || $id != '')
{
$sql = "DELETE FROM fetchmail WHERE id = '$id'";
$delete = $this->db->query($sql);
// ...
}
The logical condition ($id != 0 || $id != '')
is always true for any value of $id
, making the check essentially meaningless. The user-supplied $id
value is directly concatenated into the SQL query without sanitization, allowing SQL injection.
Multiple instances of direct string concatenation in queries throughout the save function:
For INSERT operations:
if($mda != '')
{
$sql = "INSERT INTO fetchmail (mailbox, domain, active, src_server, src_user, src_password, src_folder, poll_time, fetchall, keep, protocol, usessl, sslcertck, src_auth, mda) VALUES ('$mailbox', '$mailbox_domain', '$enabled', '$server', '$user', '$pass', '$folder', '$pollinterval', '$fetchall', '$keep', '$protocol', '$usessl', true, 'password', '$mda' )";
} else {
$sql = "INSERT INTO fetchmail (mailbox, domain, active, src_server, src_user, src_password, src_folder, poll_time, fetchall, keep, protocol, usessl, sslcertck, src_auth) VALUES ('$mailbox', '$mailbox_domain', '$enabled', '$server', '$user', '$pass', '$folder', '$pollinterval', '$fetchall', '$keep', '$protocol', '$usessl', true, 'password')";
}
$insert = $this->db->query($sql);
For UPDATE operations:
if($mda != '')
{
$sql = "UPDATE fetchmail SET mailbox = '$mailbox', domain = '$mailbox_domain', active = '$enabled', keep = '$keep', protocol = '$protocol', src_server = '$server', src_user = '$user', src_password = '$pass', src_folder = '$folder', poll_time = '$pollinterval', fetchall = '$fetchall', usessl = '$usessl', src_auth = 'password', mda = '$mda' WHERE id = '$id'";
} else {
$sql = "UPDATE fetchmail SET mailbox = '$mailbox', domain = '$mailbox_domain', active = '$enabled', keep = '$keep', protocol = '$protocol', src_server = '$server', src_user = '$user', src_password = '$pass', src_folder = '$folder', poll_time = '$pollinterval', fetchall = '$fetchall', usessl = '$usessl', src_auth = 'password' WHERE id = '$id'";
}
$update = $this->db->query($sql);
For SELECT operations:
$sql = "SELECT * FROM fetchmail WHERE mailbox='" . $mailbox . "'";
$result = $this->db->query($sql);
All of these queries have user input directly embedded into SQL strings, creating critical SQL injection vulnerabilities.
No explicit authentication verification at the beginning of action handlers, potentially allowing attackers to bypass authentication under certain conditions.
Insufficient error handling for database operations and input validation.
The plugin used base64 encoding for password storage, which is easily reversible and provides no cryptographic protection:
$pass = base64_encode(rcube_utils::get_input_value('_fetchmailpass', rcube_utils::INPUT_POST));
Several conditions used incorrect operators or logical combinations that could lead to unexpected behavior:
// This condition is always true for any $id value
if ($id != '' || $id != 0)
The following changes were made to address these vulnerabilities:
Added explicit authentication checks at the beginning of each action handler:
// Check if user is authenticated
$this->check_auth();
private function check_auth()
{
if (!$this->rc->user->ID) {
header('Location: ?_task=login');
exit;
}
}
Replaced all direct string concatenation with parameterized queries:
Original Delete Function (vulnerable):
$sql = "DELETE FROM fetchmail WHERE id = '$id'";
$delete = $this->db->query($sql);
Fixed Delete Function:
$sql = "DELETE FROM fetchmail WHERE id = ? AND mailbox = ?";
$delete = $this->db->query($sql, $id, $mailbox);
Original Insert Function (vulnerable):
if($mda != '')
{
$sql = "INSERT INTO fetchmail (mailbox, domain, active, src_server, src_user, src_password, src_folder, poll_time, fetchall, keep, protocol, usessl, sslcertck, src_auth, mda) VALUES ('$mailbox', '$mailbox_domain', '$enabled', '$server', '$user', '$pass', '$folder', '$pollinterval', '$fetchall', '$keep', '$protocol', '$usessl', true, 'password', '$mda' )";
} else {
$sql = "INSERT INTO fetchmail (mailbox, domain, active, src_server, src_user, src_password, src_folder, poll_time, fetchall, keep, protocol, usessl, sslcertck, src_auth) VALUES ('$mailbox', '$mailbox_domain', '$enabled', '$server', '$user', '$pass', '$folder', '$pollinterval', '$fetchall', '$keep', '$protocol', '$usessl', true, 'password')";
}
$insert = $this->db->query($sql);
Fixed Insert Function:
if($mda != '')
{
$sql = "INSERT INTO fetchmail (mailbox, domain, active, src_server, src_user, src_password, src_folder, poll_time, fetchall, keep, protocol, usessl, sslcertck, src_auth, mda) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)";
$insert = $this->db->query($sql, $mailbox, $mailbox_domain, $enabled, $server, $user, $pass, $folder, $pollinterval, $fetchall, $keep, $protocol, $usessl, true, 'password', $mda);
}
else
{
$sql = "INSERT INTO fetchmail (mailbox, domain, active, src_server, src_user, src_password, src_folder, poll_time, fetchall, keep, protocol, usessl, sslcertck, src_auth) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)";
$insert = $this->db->query($sql, $mailbox, $mailbox_domain, $enabled, $server, $user, $pass, $folder, $pollinterval, $fetchall, $keep, $protocol, $usessl, true, 'password');
}
Changed the conditional checks from OR to AND and implemented strict type comparison to ensure proper validation:
Original (vulnerable):
if ($id != 0 || $id != '')
Fixed:
if ($id !== 0 && $id !== '')
Added explicit handling for potentially NULL values to maintain compatibility with the database schema:
$explode_mailbox = explode('@', $mailbox);
$mailbox_user = $explode_mailbox[0] ?? $mailbox;
$mailbox_domain = !empty($explode_mailbox[1]) ? $explode_mailbox[1] : '';
...
$folder = rcube_utils::get_input_value('_fetchmailfolder', rcube_utils::INPUT_POST);
$folder = $folder ?? ''; // Default to empty string if NULL
Added user ownership checks to prevent unauthorized access to others' data:
// Using parameterized query with mailbox check for security
$sql = "DELETE FROM fetchmail WHERE id = ? AND mailbox = ?";
Implemented AES-GCM authenticated encryption for password storage instead of base64 encoding:
Original (vulnerable):
$pass = base64_encode(rcube_utils::get_input_value('_fetchmailpass', rcube_utils::INPUT_POST));
Fixed with AES-GCM encryption:
private function encrypt_password($password) {
$encryption_key = $this->rc->config->get('fetchmail_encryption_key', 'ChangeThisTestEncryptionKeyForFetchmail123');
// For AES-GCM, we need a 12-byte (96-bit) nonce/IV
$nonce = openssl_random_pseudo_bytes(12);
// Tag for authentication
$tag = '';
// Encrypt with AES-256-GCM
$encrypted = openssl_encrypt(
$password,
'aes-256-gcm',
$encryption_key,
OPENSSL_RAW_DATA,
$nonce,
$tag,
'', // No additional authenticated data
16 // Tag length: 16 bytes (128 bits)
);
// Format: base64(nonce + tag + ciphertext)
return base64_encode($nonce . $tag . $encrypted);
}
Modified Save Method:
$password = rcube_utils::get_input_value('_fetchmailpass', rcube_utils::INPUT_POST);
$pass = $this->encrypt_password($password);
Corresponding Decryption Method for fetchmail.pl:
sub decrypt_password {
my ($encrypted_data) = @_;
# Decode from base64
my $decoded = decode_base64($encrypted_data);
# Extract components
my $nonce = substr($decoded, 0, 12);
my $tag = substr($decoded, 12, 16);
my $ciphertext = substr($decoded, 28);
# Create GCM decryption object
my $gcm = Crypt::AuthEnc::GCM->new("AES", $encryption_key);
# Set the nonce (IV)
$gcm->iv_add($nonce);
# Decrypt
my $plaintext = $gcm->decrypt_add($ciphertext);
# Verify tag
my $result = $gcm->decrypt_done($tag);
if (!$result) {
syslog("err", "Authentication tag verification failed - password data may be corrupted");
return ""; # Return empty string on error
}
return $plaintext;
}
These vulnerabilities could allow an authenticated attacker to:
security@codamail.com