[pull] loki from asbru-cm:loki by pull[bot] · Pull Request #86 · RogueScholar/asbru-cm

pull · 2020-03-03T19:54:26Z

See Commits and Changes for more details.

Created by pull[bot] (v2.0.0-alpha.4)

Can you help keep this open source service alive? 💖 Please sponsor : )

… and Fedora 33

…unnel (#771) Signed-off-by: Syco <alberto.rinaudo+torann@gmail.com> Co-authored-by: Syco <alberto.rinaudo+torann@gmail.com>

…ion panel (fixes #1132, closes #1167)

…ixes #1168, closes #1169 and #1170)

sentry · 2026-04-05T15:21:25Z

+	print D "\n\n";
+	close F;
+	close D;
+	unlink $file;


Bug: The anonymize.pl script unconditionally deletes the user's original input file after creating an anonymized version, causing irreversible data loss without any warning.
_{Severity: CRITICAL}

Suggested Fix

Remove the line unlink $file; from the script to prevent the deletion of the original user file. Alternatively, make the deletion optional by adding a command-line flag like --destructive and only perform the unlink operation if that flag is present.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not valid. Location: utils/anonymize.pl#L105 Potential issue: The standalone script `utils/anonymize.pl` is intended to create an anonymized version of a user's configuration file for debugging. However, after writing the new anonymized file to a hardcoded path (`debug.yml`), the script unconditionally deletes the original input file using `unlink $file;`. There is no warning, confirmation prompt, or command-line flag to prevent this behavior. This can lead to irreversible data loss for users who run the script on their primary configuration file, as their original settings will be permanently deleted.

_{Did we get this right? 👍 / 👎 to inform future reviews.}

sentry · 2026-04-05T15:21:25Z

+    waitpid($pid,0);
+    close Reader;
+    close ErrReader;
+    if (@err) {


Bug: The code uses a shell-vulnerable form of open3() with unsanitized user input ($uid), creating a potential for shell command injection.
_{Severity: HIGH}

Suggested Fix

Use the list form of open3(), which does not invoke a shell and is not vulnerable to command injection. Pass each part of the command as a separate argument in the list, for example: open3(*Writer, *Reader, *ErrReader, $CLI, $$self{kpxc_cli}, 'show', ..., $uid). This should be applied in both testMasterKey() and getFieldValue().

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not valid. Location: lib/PACKeePass.pm#L201 Potential issue: The `open3()` function is called with a single string argument that includes an unsanitized variable, `$uid`. In Perl, this invokes a shell (`/bin/sh -c`) to execute the command, creating a command injection vulnerability. An attacker who can control KeePass entry names (which populate `$uid`) could inject malicious shell commands. For example, an entry named `test'; malicious_command; 'end` would execute `malicious_command`. While exploiting this requires prior access to the KeePass database, it is an unnecessary security risk that violates secure coding principles.

_{Did we get this right? 👍 / 👎 to inform future reviews.}

…ixes #1165, #1172)

…1174) - Display key bindings associated with both local and remote commands - Display key bindings defined in the main preferences or in the connectection itself

Detection of VTE features based on the version is not reliable

sentry · 2026-04-06T09:46:34Z

+    if ($ok eq 'ok') {
+        my $selection = $w{window}{gui}{treelist}->get_selection();
+        my $model = $w{window}{gui}{treelist}->get_model();
+        my @paths = _getSelectedRows($selection);
+        $entry = $model->get_value($model->get_iter($paths[0]),0);


Bug: The application crashes when a user clicks 'OK' in the KeePass dialog without selecting an entry, due to an unguarded access to an empty array.
_{Severity: HIGH}

Suggested Fix

Add a guard condition to verify that the @paths array is not empty before attempting to access its elements. For example: if (@paths) { ... }.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not valid. Location: lib/PACKeePass.pm#L425-L429 Potential issue: In the KeePass entry selection dialog, if a user clicks the 'OK' button without selecting an entry, the `@paths` array will be empty. The code proceeds to access `$paths[0]`, which evaluates to `undef`. This `undef` value is then passed to `$model->get_iter()`, causing the application to crash. This can occur during normal user interaction, for example, by opening the dialog and immediately clicking 'OK'.

sentry · 2026-04-06T09:46:35Z

+    } else {
+        $cfg = $self->get_cfg();
+    }
+    if (version->parse($$self{kpxc_version}) >= version->parse("2.7.0")) {


Bug: The application crashes when using the KeePass feature if keepassxc-cli is not installed, because it attempts to parse an empty version string.
_{Severity: HIGH}

Suggested Fix

Add a check in _locateEntries() to ensure $$self{kpxc_version} is not an empty string before calling version->parse(). Additionally, consider calling update() after object initialization to correctly disable UI elements when keepassxc-cli is unavailable.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not valid. Location: lib/PACKeePass.pm#L542 Potential issue: If `keepassxc-cli` is not installed, `_setCapabilities()` correctly sets `$$self{kpxc_version}` to an empty string. However, the UI state is not updated to disable the KeePass lookup button. When the user clicks this button, `_locateEntries()` is called, which attempts to parse the empty version string with `version->parse($$self{kpxc_version})`. This action throws a fatal 'Invalid version format' exception, crashing the application.

- Make it compatible with glade 3.40 - Suppress deprecated "stock icon"

sentry · 2026-04-08T16:48:43Z

+    if ($uuid && $$hk{$uuid}{$window}{$keymask}) {
+        return (0, "<i>$keymask</i> already assigned to <b>hotkey</b>\n\n$$hk{$window}{$keymask}[2]");
+    }


Bug: When a duplicate connection-specific hotkey is detected, the error message incorrectly references the global hotkey data structure, causing a warning and data corruption.
_{Severity: MEDIUM}

Suggested Fix

Update the error message on line 327 to reference the correct UUID-scoped hotkey data structure, $$hk{$uuid}{$window}{$keymask}[2], to display the correct information and prevent accessing an undefined value.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not valid. Location: lib/PACKeyBindings.pm#L326-L328 Potential issue: When checking for a duplicate connection-specific (UUID-scoped) hotkey, the code correctly identifies the conflict using `$$hk{$uuid}{$window}{$keymask}`. However, the subsequent error message incorrectly attempts to retrieve details from the global hotkey data structure, `$$hk{$window}{$keymask}[2]`, which is undefined in this context. This results in an uninitialized value warning, a misleading error message for the user, and data structure corruption due to Perl's autovivification.

sentry · 2026-04-08T16:48:43Z

+    if ($$self{cfg}) {
+        $cfg = $$self{cfg};
+    } else {
+        $cfg = $self->get_cfg();


Bug: A fatal error occurs when using KeePass substitution if the database file becomes inaccessible, as _hasCacheValue calls mtime on a undef value from stat('').
_{Severity: HIGH}

Suggested Fix

Add a check in the _hasCacheValue function to ensure that $$cfg{database} is not empty and that the stat call returns a valid object before attempting to call the mtime method on it.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not valid. Location: lib/PACKeePass.pm#L848 Potential issue: The `_hasCacheValue` function attempts to get the modification time of a KeePass database file using `stat($$cfg{database})->mtime`. If the database file is deleted or moved after the application starts, `get_cfg()` sets `$$cfg{database}` to an empty string. The subsequent call to `stat('')` returns `undef`, causing a fatal "Can't call method 'mtime' on an undefined value" error when the application tries to perform a KeePass field substitution.

sentry · 2026-04-10T09:16:37Z

+    opendir($ss, "$new_dir/screenshots");
+    while (my $f = readdir($ss)) {
+        if ($f =~ /\.png/) {
+            my $n = $f;
+            $n =~ s/pac/asbru/;
+            system "$ENV{'ASBRU_ENV_FOR_EXTERNAL'} mv -f $new_dir/screenshots/$f $new_dir/screenshots/$n";
+        }
+    }


Bug: The opendir call lacks an error check for a potentially missing screenshots directory, which will cause a runtime error when readdir is called on the resulting invalid handle.
_{Severity: HIGH}

Suggested Fix

Check the return value of opendir. If the directory is successfully opened, execute the while loop to process its contents, and then call closedir($ss) after the loop to release the directory handle.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not valid. Location: utils/pac2asbru.pl#L134-L141 Potential issue: The script calls `opendir($ss, "$new_dir/screenshots")` without validating the return value. If the `screenshots` directory does not exist, which is a valid scenario for older installations being migrated, `opendir` returns false. The subsequent `readdir($ss)` call on the invalid handle will then trigger a runtime error, interrupting the migration process and potentially leaving the application configuration in an inconsistent state. Additionally, the directory handle is not closed with `closedir` after use.

(cherry picked from commit 039d81c)

(inspired from commit 42e16ca)

(inspired by commit 073e980)

(inspired by commit fd9ab5a)

Introduced with the change of OSSP::uuid

sentry · 2026-04-11T15:31:22Z

+    if ((!defined $file)||(-d $file)||(!-e $file)) {
+        return 0;
+    }
+    if (!-e $pathcli) {
+        $pathcli = '';
+    }
+    if ($$self{disable_keepassxc}) {
+        $$self{cfg}{use_keepass} = 0;
+    }
    $$self{frame}{fcbKeePassFile}->set_filename($file);


Bug: Calling the update() method on a PACKeePass instance created with buildgui=0 will cause a crash because it attempts to access uninitialized GUI frame widgets.
_{Severity: HIGH}

Suggested Fix

Add a guard condition at the beginning of the update() method to check if the GUI was built before attempting to access any frame widgets. For example, return unless ($$self{frame}); or return unless ($$self{buildgui});.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not valid. Location: lib/PACKeePass.pm#L116-L125 Potential issue: When a `PACKeePass` object is instantiated with `buildgui=0` (e.g., for use in `PACTerminal` or `PACEdit`), the `$$self{frame}` attribute is not initialized. However, the `update()` method unconditionally attempts to access GUI widgets within this frame, such as `$$self{frame}{fcbKeePassFile}`. This dereferencing of an undefined value will cause a fatal Perl error, crashing the application. This can occur when `PACTerminal.pm` or `PACEdit.pm` call `update()` on a non-GUI `PACKeePass` object.

sentry · 2026-04-11T15:31:22Z

+                    }
+                }
+            });
            push(@menu_items, {
-                label => 'KeePassX',
-                stockicon => 'pac-keepass',
-                submenu => [
-                    {
-                        label => 'KeePassX title values',
-                        submenu => \@titles
-                    }, {
-                        label => 'KeePassX username values',
-                        submenu => \@usernames
-                    }, {
-                        label => 'KeePassX URL values',
-                        submenu => \@urls
-                    }, {
-                        label => "KeePass Extended Query",
-                        tooltip => "This allows you to select the value to be returned, based on another value's match againt a Perl Regular Expression",
-                        code => sub {$w{send}->insert_text("<KPXRE_GET_(title|username|password|url)_WHERE_(title|username|password|url)==Your_RegExp_here==>", -1, $w{send}->get_position);}
+                label => 'Add Password KeePassXC',
+                tooltip => 'KeePassXC Password',
+                code => sub {
+                    my $pos = $w{send}->get_property('cursor_position');
+                    my $selection = $PACMain::FUNCS{_KEEPASS}->listEntries($$self{_WINDOWEDIT});
+                    if ($selection) {
+                        $w{send}->insert_text("<password|$selection>", -1, $w{expect}->get_position);


Bug: The KeePassXC context menu for the 'Send' field incorrectly inserts text into the 'Expect' field's cursor position by using $w{expect}->get_position() instead of $w{send}->get_position().
_{Severity: MEDIUM}

Suggested Fix

Update the insert_text calls in the KeePassXC username and password handlers to use the cursor position from the correct widget. Change $w{expect}->get_position() to $w{send}->get_position() on lines 844 and 855.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not valid. Location: lib/edit/PACExpectEntry.pm#L844-L855 Potential issue: In the 'Send' field's right-click menu, the handlers for inserting KeePassXC username and password placeholders incorrectly use the cursor position from the 'Expect' field (`$w{expect}`) instead of the 'Send' field (`$w{send}`). This causes the placeholder text, like `<username|selection>`, to be inserted at the cursor's location in the 'Expect' field, not the 'Send' field where the user initiated the action. This behavior silently corrupts the user's configuration.

Regression introduced in #1184.

sentry · 2026-04-13T12:55:38Z

+            }
+            if ($$focus{uuid} eq $elem_uuid) {
+                $self->set_cursor($path, undef, 0);
+                return 1;
+            }


Bug: The _focusPrevious function can crash because $focus is dereferenced without being initialized if the target is the first element in the tree.
_{Severity: HIGH}

Suggested Fix

Add a check to ensure the $focus variable is defined before it is dereferenced in the second loop. For example, wrap the dereference in a conditional like if (defined $focus) { ... }.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not valid. Location: lib/ex/PACTree.pm#L259-L263 Potential issue: In the `_focusPrevious` method, the `$focus` variable is initialized to `undef`. If the target `uuid` is found on the first iteration of the initial loop, the function proceeds to a second loop without ever assigning a value to `$focus`. This second loop then unconditionally dereferences `$focus` (e.g., `$$focus{uuid}`), causing a "Can't use an undefined value as a HASH reference" runtime crash. This occurs when a user navigates up from the first item in a tree view.

Code was not working on some specific versions of perl (at least with 5.38.2 shipped with Ubuntu 24.04).

sentry · 2026-04-13T16:49:56Z

+    if ($ok eq 'ok') {
+        my $selection = $w{window}{gui}{treelist}->get_selection();
+        my $model = $w{window}{gui}{treelist}->get_model();
+        my @paths = _getSelectedRows($selection);
+        $entry = $model->get_value($model->get_iter($paths[0]),0);


Bug: In listEntries(), accessing $paths[0] without checking if the @paths array is empty causes a crash if a user clicks "ok" without selecting a KeePass entry.
_{Severity: HIGH}

Suggested Fix

Add a guard clause to verify that the @paths array is not empty after the user interacts with the dialog. If it is empty, either do nothing or show a message, but do not proceed to access the first element.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not valid. Location: lib/PACKeePass.pm#L425-L429 Potential issue: The `listEntries()` function in `lib/PACKeePass.pm` displays a dialog for KeePass entry selection. If a user clicks the "ok" button without selecting an entry from the list, the `@paths` array will be empty. The code then attempts to access `$paths[0]`, which evaluates to `undef`. This `undef` value is passed to `$model->get_iter()`, leading to a fatal runtime crash when `$model->get_value()` is called on the resulting invalid iterator. The same vulnerability exists in the `row_activated` callback.

sentry · 2026-04-13T16:49:56Z

+    my $shift   = $state * ['shift-mask']   ? 'Shift' : '';
+    my $ctrl    = $state * ['control-mask'] ? 'Ctrl'  : '';
+    my $alt     = $state * ['mod1-mask']    ? 'Alt'   : '';


Bug: The code in GetKeyMask uses multiplication instead of a bitwise AND to check for modifier keys, which will cause modifier key detection to fail.
_{Severity: HIGH}

Suggested Fix

Replace the multiplication operation with a bitwise AND (&) and use the appropriate Gdk::ModifierType constants (e.g., Gdk::ModifierType::SHIFT_MASK) to correctly check for active modifier keys in the $state variable.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not valid. Location: lib/PACKeyBindings.pm#L90-L92 Potential issue: In the new `PACKeyBindings.pm` file, the `GetKeyMask` method incorrectly checks for modifier keys like Shift, Ctrl, and Alt. It uses multiplication with an array reference (e.g., `$state * ['shift-mask']`) instead of the correct bitwise AND operation (e.g., `$state & Gdk::ModifierType::SHIFT_MASK`). This logical error will cause the detection of all modifier keys to fail, effectively breaking any keybindings that rely on them.

pull Bot assigned RogueScholar Mar 3, 2020

pull Bot added the ⤵️ pull label Mar 3, 2020

pull Bot requested a review from RogueScholar March 3, 2020 19:54

restyled-io Bot mentioned this pull request Mar 3, 2020

Restyle [pull] loki from asbru-cm:loki #87

Open

pull Bot added the merge-conflict Resolve conflicts manually label Mar 4, 2020

gfrenoy and others added 25 commits February 24, 2021 23:15

🐛 Do show main window even if initially iconified (#783)

81ad53c

🐛 Do show main window even if initially iconified (#783)

2527af9

➕ Fix dependency on ncat for older Debian-based systems (#782)

35b9487

📝 Do not mention yaourt (#784)

7264343

➕ Fix dependency on ncat for older Debian-based systems (#782)

341c30a

📝 Unify the way commands are displayed in README

9bf26d8

🐛 Ensure the main window remains hidden when starting iconified (#783)

983c6e8

🐛 Ensure the main window remains hidden when starting iconified (#783)

eb74582

🔖 Prepare release notes for 6.3.2

45d2ba5

🔖 Bump to version 6.3.3

076b790

📦 Apply coding standards (#217)

0ce2262

💄 Improve display of keybindings (#787)

c69737d

🚸 Do focus on terminal when hiding connection list (#786)

9b61291

🐛 Allow key binding on alt+c, alt+r or alt+s (#786)

dd3f3d6

🚸 Do focus on favorites when showing connections list (#786)

1e1062a

🚸 Rearrange authentication user/password and manual mode (#788)

bc6e798

✨ Move SOCKS5 random port tunnel to SSH options (#771)

27aa786

📦 Support for PopOS 20.04 (#789) + use packagecloud for Ubuntu Groovy…

710d0df

… and Fedora 33

📦 Fix docker image for PopOS 20.04 (#789)

6be4123

✨ Implement other connection options and use for SOCKS5 random port t…

4a7732f

…unnel (#771) Signed-off-by: Syco <alberto.rinaudo+torann@gmail.com> Co-authored-by: Syco <alberto.rinaudo+torann@gmail.com>

🐛 Fix error when closing exploded tabs from PCC (#791)

9ea3cb4

📝 Add documentation about the new SOCK5 random port tunnel (#771)

8d21460

🔀 Merge branch 'master' into loki

c74ff1d

🐛 Support for KeePass entry containing a pipe (|) in title (#794)

dacd5d9

🐛 Fix regression in SSH options (#795)

f0b22ad

hanspr added 3 commits April 5, 2026 16:54

📝 Document use of <<ASK_PASS>> substitution (closes #1166)

f98c505

✨ Add option "-T" (disable pseudo-terminal allocation) in SSH connect…

6a7e6c9

…ion panel (fixes #1132, closes #1167)

🩹 Allow use of special characters (eg \n) for multiple line commands (f…

ca69266

…ixes #1168, closes #1169 and #1170)

sentry Bot reviewed Apr 5, 2026

View reviewed changes

gfrenoy and others added 2 commits April 5, 2026 17:28

🔖 Bump to version 6.4.2

3461a10

✨ Support for <<ASK_PASS>> keyword for interactive password request (f…

34fd758

…ixes #1165, #1172)

sentry Bot reviewed Apr 5, 2026

View reviewed changes

Comment thread lib/method/PACMethod_ssh.pm

Comment thread lib/edit/PACTermOpts.pm

Comment thread lib/edit/PACExpectEntry.pm

hanspr and others added 2 commits April 6, 2026 11:41

🩹 Display key bindings available in the context of a terminal (close #…

0bf4fd4

…1174) - Display key bindings associated with both local and remote commands - Display key bindings defined in the main preferences or in the connectection itself

👽 Improve detection of VTE features (close #1178)

1adf5e2

Detection of VTE features based on the version is not reliable

sentry Bot reviewed Apr 6, 2026

View reviewed changes

✨ Support additional prompt for MFA (#751)

2c31802

sentry Bot reviewed Apr 6, 2026

View reviewed changes

Comment thread asbru-cm

Comment thread asbru-cm

gfrenoy added 2 commits April 6, 2026 18:28

💄 Upgrade glade file

39891a1

- Make it compatible with glade 3.40 - Suppress deprecated "stock icon"

👽 Replace OSSP::uuid by Data::UUID (fixes #1183)

338b482

sentry Bot reviewed Apr 8, 2026

View reviewed changes

🔀 Merge branch 'master' into loki

fad8920

sentry Bot reviewed Apr 10, 2026

View reviewed changes

gfrenoy added 5 commits April 10, 2026 11:33

✨ Add system dark mode auto-detection with GTK dark theme hint

22d72fb

(cherry picked from commit 039d81c)

💄 Review connections list default colors

d636d0f

(inspired from commit 42e16ca)

👽 Ensure graceful fallback when Gtk3::StatusIcon is unavailable (#1161)

61c669d

(inspired by commit 073e980)

👽 Improve Wayland/Xwayland detection and routing

1f67622

(inspired by commit fd9ab5a)

🐛 Fix error when duplicating a node

f855cf3

Introduced with the change of OSSP::uuid

sentry Bot reviewed Apr 11, 2026

View reviewed changes

🐛 Fix start of multiple selected connections (#1184)

d89f6f3

Regression introduced in #1184.

sentry Bot reviewed Apr 13, 2026

View reviewed changes

🐛 Fix zoom in/out in a terminal (fixes #1136)

a9b61cb

Code was not working on some specific versions of perl (at least with 5.38.2 shipped with Ubuntu 24.04).

sentry Bot reviewed Apr 13, 2026

View reviewed changes

Comment thread lib/method/PACMethod_ssh.pm

Comment thread lib/edit/PACTermOpts.pm

🩹 Do not collapse folder after a connection to a searched node (#1185)

184801e

sentry Bot reviewed Apr 13, 2026

View reviewed changes

🩹 Improve group sort when having the same connection names (#1184)

441bf66

Conversation

pull Bot commented Mar 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

sentry Bot Apr 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sentry Bot Apr 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sentry Bot Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

sentry Bot Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sentry Bot Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

sentry Bot Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

sentry Bot Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

sentry Bot Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

sentry Bot Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

sentry Bot Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sentry Bot Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

sentry Bot Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

20 participants

pull Bot commented Mar 3, 2020 •

edited

Loading