Merge Feature/utility library (asu cidse 2025 fall) to main test

[Daniel-Lopez246]

No description provided.

[gemini-code-assist]

Code Review

This pull request introduces a comprehensive management system for a Cybercafe hotspot, including a session daemon, infrastructure setup scripts, and a PHP-based web interface. The review identifies critical SQL injection vulnerabilities in the frontend and several technical issues in the backend scripts, such as syntax errors, unhandled NULL database results, and inefficient command execution within loops. Additionally, the feedback recommends avoiding hardcoded interface names and refining traffic redirection rules to ensure compatibility with non-HTTP services. All provided comments point to valid bugs or improvement opportunities.

[gemini-code-assist]

There is a syntax error in the SQL query due to an extra closing parenthesis ) at the end of the string. This will cause the sqlite3 command to fail.

Suggested change

	RESPONSE=$(sqlite3 "${DATABASE_PATH}" "SELECT SUM(interval_bytes_tx) FROM user_data_usage WHERE user_id='${USER_ID}' AND session_number='${SESSION_NUMBER}')") > /dev/null 2>> error.log
	RESPONSE=$(sqlite3 "${DATABASE_PATH}" "SELECT SUM(interval_bytes_tx) FROM user_data_usage WHERE user_id='${USER_ID}' AND session_number='${SESSION_NUMBER}'" 2>> error.log)

[gemini-code-assist]

The $username variable is concatenated directly into the SQL query, which makes the application vulnerable to SQL injection. Use prepared statements to safely handle user input.

	$stmt = $db->prepare("SELECT 1 FROM users WHERE username=:username");
	$stmt->bindValue(':username', $username, SQLITE3_TEXT);
	$response = $stmt->execute();
	$responseArray = $response->fetchArray();
	if($responseArray)

[gemini-code-assist]

The session_id from cookies is used directly in a query without sanitization or prepared statements, leading to a SQL injection vulnerability.

		$db = global_createDatabaseObj();
		$stmt = $db->prepare("SELECT user_id FROM internet_sessions WHERE session_id=:session_id");
		$stmt->bindValue(':session_id', $cookies['session_id'], SQLITE3_TEXT);
		$response = $stmt->execute();
		$responseArray = $response->fetchArray();

[gemini-code-assist]

If the SUM function returns NULL (which happens if no rows match the criteria), the sqlite3 command will output an empty string. This causes a syntax error in Bash arithmetic expansion $((...)). Use COALESCE or IFNULL in the SQL query to ensure a numeric value is always returned.

Suggested change

	TOTAL_TX_DAY=$(($(sqlite3 "${DATABASE_PATH}" "SELECT SUM(interval_bytes_tx) FROM user_data_usage WHERE user_id='${USER_ID}' AND entry_datetime>=datetime(datetime(),'localtime','-1 days')"))) > /dev/null
	TOTAL_RX_DAY=$(($(sqlite3 "${DATABASE_PATH}" "SELECT SUM(interval_bytes_rx) FROM user_data_usage WHERE user_id='${USER_ID}' AND entry_datetime>=datetime(datetime(),'localtime','-1 days')"))) > /dev/null
	TOTAL_TX_WEEK=$(($(sqlite3 "${DATABASE_PATH}" "SELECT SUM(interval_bytes_tx) FROM user_data_usage WHERE user_id='${USER_ID}' AND entry_datetime>=datetime(datetime(),'localtime','-7 days')"))) > /dev/null
	TOTAL_RX_WEEK=$(($(sqlite3 "${DATABASE_PATH}" "SELECT SUM(interval_bytes_rx) FROM user_data_usage WHERE user_id='${USER_ID}' AND entry_datetime>=datetime(datetime(),'localtime','-7 days')"))) > /dev/null
	TOTAL_TX_MONTH=$(($(sqlite3 "${DATABASE_PATH}" "SELECT SUM(interval_bytes_tx) FROM user_data_usage WHERE user_id='${USER_ID}' AND entry_datetime>=datetime(datetime(),'localtime','-30 days')"))) > /dev/null
	TOTAL_RX_MONTH=$(($(sqlite3 "${DATABASE_PATH}" "SELECT SUM(interval_bytes_rx) FROM user_data_usage WHERE user_id='${USER_ID}' AND entry_datetime>=datetime(datetime(),'localtime','-30 days')"))) > /dev/null
	TOTAL_TX_DAY=$(sqlite3 "${DATABASE_PATH}" "SELECT COALESCE(SUM(interval_bytes_tx), 0) FROM user_data_usage WHERE user_id='${USER_ID}' AND entry_datetime>=datetime(datetime(),'localtime','-1 days')")
	TOTAL_RX_DAY=$(sqlite3 "${DATABASE_PATH}" "SELECT COALESCE(SUM(interval_bytes_rx), 0) FROM user_data_usage WHERE user_id='${USER_ID}' AND entry_datetime>=datetime(datetime(),'localtime','-1 days')")
	TOTAL_TX_WEEK=$(sqlite3 "${DATABASE_PATH}" "SELECT COALESCE(SUM(interval_bytes_tx), 0) FROM user_data_usage WHERE user_id='${USER_ID}' AND entry_datetime>=datetime(datetime(),'localtime','-7 days')")
	TOTAL_RX_WEEK=$(sqlite3 "${DATABASE_PATH}" "SELECT COALESCE(SUM(interval_bytes_rx), 0) FROM user_data_usage WHERE user_id='${USER_ID}' AND entry_datetime>=datetime(datetime(),'localtime','-7 days')")
	TOTAL_TX_MONTH=$(sqlite3 "${DATABASE_PATH}" "SELECT COALESCE(SUM(interval_bytes_tx), 0) FROM user_data_usage WHERE user_id='${USER_ID}' AND entry_datetime>=datetime(datetime(),'localtime','-30 days')")
	TOTAL_RX_MONTH=$(sqlite3 "${DATABASE_PATH}" "SELECT COALESCE(SUM(interval_bytes_rx), 0) FROM user_data_usage WHERE user_id='${USER_ID}' AND entry_datetime>=datetime(datetime(),'localtime','-30 days')")

[gemini-code-assist]

Executing iptables -L inside a loop for every internet session is extremely inefficient and will cause significant performance degradation as the number of sessions grows. Additionally, the grep command might match multiple lines if an IP address is a substring of another (e.g., 192.168.1.1 matching 192.168.1.10), leading to incorrect results.

Suggested change

	SESSION_TX=$(($(iptables -t mangle -L iptmon_tx -vxn | grep "${USER_IP}" | awk '{print $2}')))
	SESSION_RX=$(($(iptables -t mangle -L iptmon_rx -vxn | grep "${USER_IP}" | awk '{print $2}')))
	SESSION_TX=$(iptables -t mangle -L iptmon_tx -vxn | awk -v ip="${USER_IP}" '$0 ~ " "ip" " {print $2}')
	SESSION_RX=$(iptables -t mangle -L iptmon_rx -vxn | awk -v ip="${USER_IP}" '$0 ~ " "ip" " {print $2}')

[gemini-code-assist]

This rule redirects all TCP traffic on the hotspot interface to port 80. This will break HTTPS (port 443) and any other TCP-based services. It is recommended to restrict this redirection to port 80 only.

Suggested change

	iptables -t nat -I PREROUTING 1 -p tcp -i "${HS_INTERFACE}" \
	-m comment --comment "cybercafe-dnat" \
	-j DNAT --to-destination "${LOCAL_IP}:80"
	iptables -t nat -I PREROUTING 1 -p tcp --dport 80 -i "${HS_INTERFACE}" \
	-m comment --comment "cybercafe-dnat" \
	-j DNAT --to-destination "${LOCAL_IP}:80"

[gemini-code-assist]

The trap command is defined inside the while true loop, which means it is re-registered on every iteration. It should be moved outside the loop for better performance and clarity.

Suggested change

	while true; do
	trap 'echo -e "$(date) Error in Cybercafe_daemon.sh: Line ${LINENO}\n" >> error.log' ERR > /dev/null 2>> error.log
	trap 'echo -e "$(date) Error in Cybercafe_daemon.sh: Line ${LINENO}\n" >> error.log' ERR > /dev/null 2>> error.log
	while true; do

[gemini-code-assist]

This loop iterates from 0 to the maximum table_index. If there are large gaps in the indices, this approach is highly inefficient as it calls remove_session for many non-existent entries. A better approach would be to fetch the list of active indices and iterate over them.

[gemini-code-assist]

The network interface wlan0 is hardcoded here. It should use the $HS_INTERFACE variable defined in the configuration file to ensure consistency and portability.

Suggested change

	ip add show dev wlan0 | grep 192\.168\.43\. > /dev/null 2>> error.log # Does it appear the hotspot is active?
	ip add show dev "$HS_INTERFACE" | grep 192\.168\.43\. > /dev/null 2>> error.log # Does it appear the hotspot is active?