Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apostrophe in SSID generates an invalid firstrun.sh #486

Closed
mjc opened this issue Sep 22, 2022 · 13 comments
Closed

Apostrophe in SSID generates an invalid firstrun.sh #486

mjc opened this issue Sep 22, 2022 · 13 comments

Comments

@mjc
Copy link

mjc commented Sep 22, 2022

firstrun.sh does not correctly escape single quote in WiFi SSIDs, leading to a pi that simply powers off on first boot.

There's a few other similar things that shellcheck catches in there, also.

@nickschot
Copy link

Ran into this as well!

@rbsamoht
Copy link

rbsamoht commented Oct 12, 2022

Same issue here, using an apostrophe character (') in the SSID will generate an invalid bash output in the firstrun.sh file.

The apostrophe is not properly escaped:

'mySSIDWith\'Apostrophe'

Something like '\'' should probably be used instead:

'mySSIDWith'\''Apostrophe'

Environment

  • Windows 11
  • Raspberry Pi Imager v1.7.3

Current workaround

Downgrade to Raspberry Pi Imager v1.7.2, that doesn't seem to be affected by this issue

@tomstuart
Copy link

This bit me too. My workaround was to manually edit line 42 in the generated firstrun.sh to use double (vs single) quotes around the SSID.

@tomstuart
Copy link

The problem was introduced in 4961990. It tries to use the escapeshellarg function to escape the SSID, but that function doesn’t escape single quotes correctly for Bash (“A single quote may not occur between single quotes, even when preceded by a backslash.”). As @rbsamoht says above, it’d be better to replace ' with '\'' so that the string is split into two parts which are joined with an unquoted apostrophe.

maxnet added a commit that referenced this issue Oct 15, 2022
@livvy94
Copy link

livvy94 commented Oct 28, 2022

I just ran into this! For fellow googlers out there, here's the exact error message on boot:

[   23.217672] firstrun.sh[309]: /boot/firstrun.sh: line 70: unexpected EOF while looking for matching '"'
[   23.219300] firstrun.sh[309]: /boot/firstrun.sh: line 70: syntax error: unexpected end of file

At this point, it powers down.

[  OK  ] Reached target Power-Off.

@Longsight
Copy link

The problem was introduced in 4961990. It tries to use the escapeshellarg function to escape the SSID, but that function doesn’t escape single quotes correctly for Bash (“A single quote may not occur between single quotes, even when preceded by a backslash.”). As @rbsamoht says above, it’d be better to replace ' with '\'' so that the string is split into two parts which are joined with an unquoted apostrophe.

Probably an even cleaner solution would be to prefix the string with $, forcing ANSI-C quoting and allowing \' to work as expected, eg:

/usr/lib/raspberrypi-sys-mods/imager_custom set_wlan 'Don\'tgonearthe Castle' - fails
/usr/lib/raspberrypi-sys-mods/imager_custom set_wlan $'Don\'tgonearthe Castle' - works

Though having said that I notice escapeshellargs has already been fixed with the '"'"' approach, which does work.

@maxnet
Copy link
Collaborator

maxnet commented Dec 11, 2022

Two (fairly obvious) workarounds:

Another option is to try 1.7.4
Not officially released yet, but you can grab a build here: Windows Linux Mac

@nathan-hook
Copy link

Two (fairly obvious) workarounds:

Another option is to try 1.7.4 Not officially released yet, but you can grab a build here: Windows Linux Mac

I ended up deleting my comment because the second workaround actually didn't work.

But am happy to report that the 1.7.4 version of the imager seems to have fixed the problem.

@vollkorn1982
Copy link

I ran into this problem as well and my solution was to change my SSID to the hex representation, similarly as is already done with the passphrase. This seemed the safest option to me, because no quoting mistakes can be made and it definitely avoids shell injection attacks or similar problems.

https://web.mit.edu/freebsd/head/contrib/wpa/wpa_supplicant/wpa_supplicant.conf says:

# ssid: SSID (mandatory); network name in one of the optional formats:
#	- an ASCII string with double quotation
#	- a hex string (two characters per octet of SSID)
#	- a printf-escaped ASCII string P"<escaped string>"

@maxnet
Copy link
Collaborator

maxnet commented Dec 13, 2022

and it definitely avoids shell injection attacks or similar problems.

Well, on Windows, OSX and Linux, any user (not just root/administrator) can access files on the FAT partition of removable drives.
So one does not need to (ab)use Imager to inject shell commands. Can simply open firstrun.sh in notepad and add them there. :-)

@vollkorn1982
Copy link

Hi @maxnet, to be honest, your answer really worked me up. Let me explain why.

I gave a hint about how to build this feature more secure, but instead of approaching this possibility you brushed it away. Yes, your threat model may make this not a necessary security improvement, but other people's threat models may. I won't go into a discussion about under which conditions it could be a valid security issue. These discussions usually only waste time and energy magnitudes higher that it would've to simply implement the more secure way.

Instead, I'll let you know what your message communicated to me.

  1. "Yeah, I know this is an insecure coding style, but I don't care." This is cultivating insecure coding habits. I now believe you're ok with building up technical debt.

  2. I think this will tell other people working on this project, that it's fine to knowingly write insecure code as long as they find a plausible excuse not to do it proper.

  3. Brushing off a security advice like this may also signal to external people reading here, that it's probably pointless to tell you about their security concerns and findings, because they would have to convince you first, instead of you being thankful for helping to improve the project. With regards to security, this can be pretty bad.

Now, quite honestly, writing this post might already have taken longer than looking up how the hex encoding of the password is done and applying it to the SSID as well. So, maybe I am guilty of my own complaint, too, but maybe my message resonates with you and I made this world a little bit better.

I'll try to find the corresponding code and make a PR now.

@maxnet
Copy link
Collaborator

maxnet commented Dec 13, 2022

I'll try to find the corresponding code and make a PR now.

Your PR is unlikely to work without new RPI OS images...
With the move to NetworkManager in mind, setting wifi password is outsourced to a /usr/lib/raspberrypi-sys-mods/imager_custom script inside RPI OS nowadays, which in turns call raspi-config and nmcli.
Wouldn't count on those accepting hexadecimal SSIDs right now.
(think nmcli/NetworkManager in turn configures wpa_supplicant through DBus calls, and not with wpa_supplicant.conf, but haven't checked)

And no, doing configuration by passing user input as parameter to a script (that resulted in this issue in the first place) wasn't my personal preference either. And yes, I am aware that comes with other issues than just escaping on multi-user systems.
Wasn't my decision.

@cillian64
Copy link
Collaborator

After RPi-Distro/raspberrypi-sys-mods#83 ' works in SSIDs and PSKs (as well as all the other interesting characters I could think of to try)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants