HiveBrain v1.2.0
Get Started
← Back to all entries
patternbashMinor

Checking dates against a .properties file

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
filepropertiesdatescheckingagainst

Problem

I am writing a script that will check the set values against a .properties file and I am just wondering if there is any nicer way to write what I have here.

#!/bin/bash 
# @summary Check Values 
# --------------------------------------------------------------------------------------------------
function _RunChecks
{
SuccessfulDiffRun="true" 
timestamp() { date +"%F"; }
TodaysDate=$(timestamp)
#Read savedState.properties file
while read LINE; do eval $LINE; done < savedState.properties
#Compare the Values
echo "HotPatch : "
if [ "$TodaysDate" = "$WD_MANAGEGOLD_DATETIMESTAMP" ]; then
    echo Dates Are A Match : TodaysDate:$TodaysDate = HotpatchRunDate:$WD_MANAGEGOLD_DATETIMESTAMP
    if [ "$SuccessfulDiffRun" = "$WD_MANAGEGOLD_SUCCESS" ]; then
        echo SuccessfulDiffRun : $WD_MANAGEGOLD_SUCCESS
        echo Hotpatch Run Was Successful 
    else
        echo SuccessfulDiffRun : $WD_MANAGEGOLD_SUCCESS
        echo Exiting
        exit 0
    fi
else
    echo Dates Not A Match: TodaysDate:$TodaysDate = HotpatchRunDate:$WD_MANAGEGOLD_DATETIMESTAMP
    exit 0 
fi
echo ""
echo "RC Prod  : "
if [ "$TodaysDate" = "$WD_MANAGERC_DATETIMESTAMP" ]; then
    echo Dates Are A Match : TodaysDate:$TodaysDate = RCRunDate:$WD_MANAGERC_DATETIMESTAMP
    if [ "$SuccessfulDiffRun" = "$WD_MANAGERC_SUCCESS" ]; then
        echo SuccessfulDiffRun : $WD_MANAGERC_SUCCESS
        echo RC Run Was Successful
    else
        echo SuccessfulDiffRun : $WD_MANAGERC_SUCCESS
        echo Exiting
        exit 0
    fi
else
    echo Dates Not A Match: TodaysDate:$TodaysDate = HotpatchRunDate:$WD_MANAGEGOLD_DATETIMESTAMP
    exit 0 
fi
}
# --------------------------------------------------------------------------------------------------
# @summary Start
_RunChecks

Solution

Security concerns

Judging by the eval, it seems the properties file contains assignment statements that bash understands. Or so you think. Using eval this way is most often a serious security risk. What if somebody fobs a way to tamper with the file and append a line like this:

bash /tmp/evilscript.sh


Not only the evil script will get executed by whoever is the effective user (hopefully not root), you will not even realize it happened.

Even if you're not particularly concerned by security issues (dangerous mindset though), it's just not worth the risk. One way to make this safer is to match the live against the expected format using the [[...]] built-in, or similar.

Performance

It seems there are just two lines to parse. At this scale, I doubt performance is a serious concern, or that alternative approaches would make a noticeable difference.

Simplify

I find the way you compare the previous and current values of the two variables overly tedious. I would take a much simpler, lazier approach. For example, keep a copy of the previous version of the file at a place not editable by its current users, and replace the tedious checks for each variable with a simple diff.

Code Snippets

bash /tmp/evilscript.sh

Context

StackExchange Code Review Q#91398, answer score: 4

Revisions (0)

No revisions yet.