Skip to content
Permalink
Browse files Browse the repository at this point in the history
reworked fileupload.ccc in 0031-WebUI-Fix-FileUpload WebUI patch to
contain several security checks for a valid admin session id and
query string checks as well as omitting the critical use of "eval"
to parse the URL query string altogether. This should significantly
improve the security burden, thus fix a security issue raised by @qx-f7
  • Loading branch information
jens-maus committed Mar 14, 2022
1 parent 29e4ff5 commit 3485465
Show file tree
Hide file tree
Showing 2 changed files with 111 additions and 15 deletions.
64 changes: 56 additions & 8 deletions buildroot-external/patches/occu/0031-WebUI-Fix-FileUpload.patch
Expand Up @@ -365,12 +365,18 @@
}
--- occu/WebUI/www/config/fileupload.ccc.orig
+++ occu/WebUI/www/config/fileupload.ccc
@@ -0,0 +1,61 @@
@@ -0,0 +1,109 @@
+#!/bin/sh
+# shellcheck shell=dash disable=SC2169,SC2034,SC2154
+# shellcheck shell=dash disable=SC3036,SC3010,SC2034,SC3060,SC2116,SC3045 source=/dev/null
+
+echo -ne "Content-Type: text/html; charset=iso-8859-1\r\n\r\n"
+
+# allow only POST requests
+if [[ "${REQUEST_METHOD}" != "POST" ]]; then
+ echo "ERROR: no POST request"
+ exit 1
+fi
+
+# fake read boundary+disposition, etc.
+read -r boundary
+read -r disposition
Expand All @@ -387,21 +393,63 @@
+# 6 + 2 newlines == 10 junk bytes
+a=$((a*2+b+c+d+10))
+
+# extract all params from QUERY_STRING
+# shellcheck disable=SC2046,SC2116,SC2086
+eval $(echo ${QUERY_STRING//&/;})
+
+# calculate the expected content length using
+# HTTP_CONTENT_LENGTH or CONTENT_LENGTH
+if [ -z "${HTTP_CONTENT_LENGTH}" ]; then
+if [[ -z "${HTTP_CONTENT_LENGTH}" ]]; then
+ HTTP_CONTENT_LENGTH=${CONTENT_LENGTH}
+fi
+SIZE=$((HTTP_CONTENT_LENGTH-a))
+
+# continue only if SIZE > 0
+if [[ "${SIZE}" -le 0 ]]; then
+ echo "ERROR: POST size <= 0"
+ exit 1
+fi
+
+# extract known params from QUERY_STRING only
+while IFS= read -r -d '&' KEYVAL && [[ -n "$KEYVAL" ]]; do
+ case ${KEYVAL%%=*} in
+ url) url=${KEYVAL#*=} ;;
+ sid) sid=${KEYVAL#*=} ;;
+ action) action=${KEYVAL#*=} ;;
+ downloadOnly) downloadOnly=${KEYVAL#*=} ;;
+ esac
+done <<END
+$(echo "${QUERY_STRING}&")
+END
+
+# check for url and action parameter
+if [[ -z "${url}" ]] || [[ -z "${action}" ]]; then
+ echo "ERROR: missing required URL parameters"
+ exit 1
+fi
+
+# check for a valid ADMIN session id
+if [[ "${#sid}" -eq 12 ]]; then
+ # parse the current version
+ [[ -r /VERSION ]] && . /VERSION
+
+ # use CCU.getVersion which is allowed only for Admins
+ RES=$(/usr/bin/curl http://127.0.0.1/api/homematic.cgi \
+ -H 'Content-Type: application/json' \
+ -d "{\"method\":\"CCU.getVersion\",\"params\":{\"_session_id_\": \"${sid//@}\"}}")
+
+ # check the curl result contains the current
+ # version number or not
+ if ! echo "${RES}" | grep -q "${VERSION}"; then
+ echo "ERROR: no valid admin session id"
+ exit 1
+ fi
+else
+ echo "ERROR: invalid session id"
+ exit 1
+fi
+
+# write out the data
+filename=$(mktemp -p /usr/local/tmp)
+if ! /usr/bin/head -q -c ${SIZE} >"${filename}"; then
+ echo "ERROR (head)"
+ echo "ERROR: head failure"
+ exit 1
+fi
+
+echo "<html>"
Expand Down
@@ -1,8 +1,14 @@
#!/bin/sh
# shellcheck shell=dash disable=SC2169,SC2034,SC2154
# shellcheck shell=dash disable=SC3036,SC3010,SC2034,SC3060,SC2116,SC3045 source=/dev/null

echo -ne "Content-Type: text/html; charset=iso-8859-1\r\n\r\n"

# allow only POST requests
if [[ "${REQUEST_METHOD}" != "POST" ]]; then
echo "ERROR: no POST request"
exit 1
fi

# fake read boundary+disposition, etc.
read -r boundary
read -r disposition
Expand All @@ -19,21 +25,63 @@ d=0
# 6 + 2 newlines == 10 junk bytes
a=$((a*2+b+c+d+10))

# extract all params from QUERY_STRING
# shellcheck disable=SC2046,SC2116,SC2086
eval $(echo ${QUERY_STRING//&/;})

# calculate the expected content length using
# HTTP_CONTENT_LENGTH or CONTENT_LENGTH
if [ -z "${HTTP_CONTENT_LENGTH}" ]; then
if [[ -z "${HTTP_CONTENT_LENGTH}" ]]; then
HTTP_CONTENT_LENGTH=${CONTENT_LENGTH}
fi
SIZE=$((HTTP_CONTENT_LENGTH-a))

# continue only if SIZE > 0
if [[ "${SIZE}" -le 0 ]]; then
echo "ERROR: POST size <= 0"
exit 1
fi

# extract known params from QUERY_STRING only
while IFS= read -r -d '&' KEYVAL && [[ -n "$KEYVAL" ]]; do
case ${KEYVAL%%=*} in
url) url=${KEYVAL#*=} ;;
sid) sid=${KEYVAL#*=} ;;
action) action=${KEYVAL#*=} ;;
downloadOnly) downloadOnly=${KEYVAL#*=} ;;
esac
done <<END
$(echo "${QUERY_STRING}&")
END

# check for url and action parameter
if [[ -z "${url}" ]] || [[ -z "${action}" ]]; then
echo "ERROR: missing required URL parameters"
exit 1
fi

# check for a valid ADMIN session id
if [[ "${#sid}" -eq 12 ]]; then
# parse the current version
[[ -r /VERSION ]] && . /VERSION

# use CCU.getVersion which is allowed only for Admins
RES=$(/usr/bin/curl http://127.0.0.1/api/homematic.cgi \
-H 'Content-Type: application/json' \
-d "{\"method\":\"CCU.getVersion\",\"params\":{\"_session_id_\": \"${sid//@}\"}}")

# check the curl result contains the current
# version number or not
if ! echo "${RES}" | grep -q "${VERSION}"; then
echo "ERROR: no valid admin session id"
exit 1
fi
else
echo "ERROR: invalid session id"
exit 1
fi

# write out the data
filename=$(mktemp -p /usr/local/tmp)
if ! /usr/bin/head -q -c ${SIZE} >"${filename}"; then
echo "ERROR (head)"
echo "ERROR: head failure"
exit 1
fi

echo "<html>"
Expand Down

0 comments on commit 3485465

Please sign in to comment.