Modify

Opened 8 years ago

Closed 3 years ago

#5327 closed defect (wontfix)

ddns-scripts bug with sed and html escaping

Reported by: ericew@… Owned by: developers
Priority: normal Milestone: Kamikaze 8.09.2
Component: packages Version: Kamikaze 8.09
Keywords: Cc:

Description

Because options are filtered through sed to generate the final_url string any option that contains a forward slash will cause the sed replacement on line 253 of dynamic_dns_updater.sh to fail and you end up with a blank final_url and no dynamic dns update. If your options contain url escape codes there could also be other unintentional side effects or errors.

I have replaced the code with the following which solves both problems

		for option_var in $ALL_OPTION_VARIABLES
		do
			replace_name=$(echo "\[$option_var\]" | tr 'a-z' 'A-Z')
			replace_value=$(eval echo "\$$option_var")
			replace_value=`echo $replace_value | sed -f /usr/lib/ddns/url_escape.sed`
			final_url=$(echo $final_url | sed s^"$replace_name"^"$replace_value"^g )
		done	

And here is the url_escape.sed file

# sed url escaping
s: :%20:g
s:<:%3C:g
s:>:%3E:g
s:#:%23:g
s:%:%25:g
s:{:%7B:g
s:}:%7D:g
s:|:%7C:g
s:\\:%5C:g
s:\^:%5E:g
s:~:%7E:g
s:\[:%5B:g
s:\]:%5D:g
s:`:%60:g
s:;:%3B:g
s:/:%2F:g
s:?:%3F:g
s^:^%3A^g
s:@:%40:g
s:=:%3D:g
s:&:%26:g
s:\$:%24:g

Attachments (1)

ddns-scripts-bug5327.patch (1.3 KB) - added by Eric Warnke <ericew@…> 8 years ago.
Patch against svn

Download all attachments as: .zip

Change History (7)

Changed 8 years ago by Eric Warnke <ericew@…>

Patch against svn

comment:1 Changed 8 years ago by nico

  • Milestone changed from Kamikaze to Kamikaze 8.09.2
  • Resolution set to fixed
  • Status changed from new to closed

Applied in [17798], thanks!

comment:2 Changed 8 years ago by darko.sokolic@…

I run version 17839 (bleeding enge as of today) and I this fix is not applied there...
Did I miss something regarding versioning?

By the way, I encountered the same problem even without intentionally using special regex characters in options, becouse one one option update_url is always full of these characters.
It looks like this:
http://[USERNAME]:[PASSWORD]@members.dyndns.org/nic/update?hostname=[DOMAIN]&myip=[IP]
and when sed fails when it tries to swallow it.

My fix was to avoid processing this options, so I've changed /usr/lib/ddns/dynamic_dns_updater.sh
to add a test before the line that invokes sed:
[ "${replace_name}" != "\[UPDATE_URL\]" ] && final_url=$(echo $final_url | sed s/"$replace_name"/"$replace_value"/g )

(original line text was everything after &&)

comment:3 Changed 8 years ago by nico

The fix was added to both trunk and 8.09, did you forget updating your packages repository?

comment:4 Changed 8 years ago by darko.sokolic@…

Yes, that's it; after (in trunk/scripts) feeds update -a I see new files.
Sorry for wasting time. Thanks!

comment:5 Changed 6 years ago by lilbigman@…

  • Resolution fixed deleted
  • Status changed from closed to reopened

Hi, I just noted in Backfire (10.03, r20728), that my password which was something like this (exchanged numbers and characters):

X(7{U}R)G?^G

Did not work and gave me a "bad auth". After changing to something with characters and numbers, only the script worked again.

I found that the escape characters for "(" -> "%28" and ")" -> "%29" were missing in url_escape.sed, but after adding this I still got the "bad auth".

comment:6 Changed 3 years ago by florian

  • Resolution set to wontfix
  • Status changed from reopened to closed

Can you file a bug against https://github.com/openwrt/packages if that is still a problem?

Add Comment

Modify Ticket

Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.