Making a script secure to code injection

Heyas

I've been told my scipts would be insecure, and to fix that.
Figured i might rethink some parts of my coding style, meanwhile i tried to write an additional catcher.

After reading:
fail : Security Issues - didnt help too much, infact - it confused me even more.
n/a: Shellshock - Wikipedia - right topic, wrong approach
n/a: Type system - Wikipedia - wrong topic, hint for better approach
ok : https://en.wikipedia.org/wiki/Code\_injection\#Shell_injection - that is something i was looking for

I came up with this:

#!/bin/bash
break_on_injections() { # "$1"
# Pass the first argument, and compare vs malware lists
# 
	[ -z "$1" ] && return 1
	checkthis="${1}"
	malware='; � | < & > :'
	#malware2=( '$(' '&&' )
	for mal in $malware #"${malware2[@]}"
	do	mal=$(echo "$mal" | sed s," ","",g)
		echo "$checkthis" | grep "$mal" && \
			printf "%s\t%s\n" \
				"Aborting, injection detected!" "" \
				"Source: " "$(echo ${BASH_SOURCE[@]})" \
				"Function:" "$(echo ${FUNCNAME[@]})" \
				"Pipestatus:" "$(echo ${PIPESTATUS[@]})" && \
				return 0
	done
	return 1
}

break_on_injections "$1" && exit 1

exit
#########################################
#########################################
#
#	Recomended usage:
#
#########################################
#########################################
while arg in "${@}"
do	break_on_injections "$arg" && exit 1
done

My 'test' result seemed good, i guess - but since i dont know much about these kind of things, i'm not sure if its even applicable.

0 $ inject.sh "$(hostname)"

0 $ inject.sh || echo hmm
hmm

1 $ inject.sh "|| echo hmm"
|| echo hmm
Aborting, injection detected!	
Source: 	/bin/inject.sh /bin/inject.sh
Function:	break_on_injections main
Pipestatus:	0 0

As i am completely inexperienced with the topic of 'code injection' - or how to protect a script from such - i'm quite happy with the above code.
Though, the "$(hostname)" didnt raise an issue, but i guess that is something one cannot catch on that level.

Is this code snippet 'usable'?
Any thoughts / suggestions / advice / improvements?

"$(hostname)" is substituted - by the calling shell!
'$(hostname)' will pass it to the script as is.

A follow up question... (and partialy missing explanation)

If you have a webserver which is using scripts to (print to a 'html-file' to) display information.
Where would you/I need to catch the code injection?

  1. Between the surfer and the page (html -> cgi/php)
  2. Between the server and the script (cgi/php -> shell)
  3. The script has to do all the possible captures (shell)

---------- Post updated at 08:02 ---------- Previous update was at 08:00 ----------

I do understand the words, but not in regards of security.
As i understand it, i'd have NO way to possible catch that?

EDIT: Or are you saying its a nonsense test?

I mean you must test it with

inject.sh '$(hostname)'
1 Like

Ok, that part (using the single quotes) is handled fine, as far i tested/'understand this topic'.

Any additions i should add?

I would code that differently:

break_on_injections() {
  typeset arg ac=0
  for arg do
    ac=$((ac+=1))
    case $arg in
    (*[';`|<&>:']*|*'$('*)
      printf "%s\n" \
        "Aborting, injection detected! arg${ac}='$arg'"
      return 1
    ;;
    esac
  done
  return 0
}
break_on_injections "$@"
1 Like

To make it resistant to injections, don't use backticks anywhere.

1 Like

If your code is having injection problems wouldn't it also break strings like this Didn't you give him $5 pocket money this week?

It seems like you have some eval, $() or backtick processing against passed strings.

1 Like

@ Corona: Just to be sure, backticks are ` ~= $(cmd) ?
@ Chubler: Not yet, break_on_injections will be my first 'general parsing of input' of passed strings within all (most at least) my tui-commands.

From the perspektive of a local user, i'm not aware of injection problems causing more trouble than the local access to the machine anyway.
The main reason i'm currently at this, is because of a feedback when passing strings from a webpage to some (unnamed) tui-command.
In the combination, i hope to keep up the current functionality, and make it more secure.

What i figured so far, and what Rudic confirmed, was that regular strings (quoted inside " ), work as expected, but if using hardcoded strings (quoted inside ' ) will 'trigger' the break_on_injections.

To my understanding, such strings (when passed from a webpage - which tui was not thought/coded for) would (should?) be passed as hardcoded strings, or get properly escaped before passing the the tui-command, using the 'php escape functions' as proposed on the linked wiki page from the first post.

What i thought of was:
1) Make the function available to all the commands, eg: place it in a file that is source anyway.
2) Call the function before the (string-)arguments are processed any further
3) If 'injections' are found, abort with failure.

Going to try this evening.

Thank you