Bug 1227534

Summary: Bad bash expression in NeworkManager script 20-chrony
Product: [openSUSE] openSUSE Distribution Reporter: Daniel Griscom <griscom>
Component: BasesystemAssignee: E-mail List <screening-team-bugs>
Status: NEW --- QA Contact: E-mail List <qa-bugs>
Severity: Minor    
Priority: P5 - None    
Version: Leap 15.5   
Target Milestone: ---   
Hardware: All   
OS: openSUSE Leap 15.5   
Whiteboard:
Found By: --- Services Priority:
Business Priority: Blocker: ---
Marketing QA Status: --- IT Deployment: ---

Description Daniel Griscom 2024-07-08 23:56:37 UTC
I’ve found a bug in the copy of the /etc/NetworkManager/dispatcher.d/20-chrony script that comes with Leap 15.5.

The script has a command, "$chronyc onoffline > /dev/null 2>&1", which is supposed to run whenever the script is called with $2 having the value of "up" or "down". To that end, there's a script line that filters out all but "up" and "down" network events:

> # For NetworkManager consider only up/down events
> [ $# -ge 2 ] && [ "$2" != "up" ] && [ "$2" != "down" ] && exit 0

Problem: that first term, "[ $# -ge 2 ] &&", should be removed. Its only effect is to prevent "exit 0" from running if there are fewer than 2 arguments to the script. But, that only ensures that if 20-chrony is called with fewer than 2 arguments then the "$chronyc onoffline > /dev/null 2>&1" line executes, which is certainly not what the author intended.
Comment 1 Daniel Griscom 2024-07-09 09:07:00 UTC
Reviewing the Network Manager dispatcher doc, these scripts are always called with two arguments, meaning this term will never fail, and won’t cause problems in real life. But it’s confusing, as it implies the wrong thing, and if the NetworkManager script API ever changes, this latent problem may actually cause trouble. In any case, code that serves no purpose should be removed.