Project

Profile

Help

HostedRedmine.com has moved to the Planio platform. All logins and passwords remained the same. All users will be able to login and use Redmine just as before. Read more...

Bug #881773

the packet string parser is not adapted to JSON strings

Added by christian montanari about 2 years ago. Updated 5 months ago.

Status:
New
Priority:
Low
Category:
Server
Sprint/Milestone:
Start date:
2020-07-19
Due date:
% Done:

10%

Estimated time:
10.00 h

Description

here "%2Fset%20mapsize%20%22FULLSIZE%22" is received at server side but decoded as 'set mapsize'...:

5: [T000 - 2020/07/18 20:08:25] in get_packet_from_connection_json() [/usr/src/freeciv/common/networking/packets_json.c::139]: Json in: {"pid":26,"fields":[1],"message":*"%2Fset%20mapsize%20%22FULLSIZE%22"*}
5: [T000 - 2020/07/18 20:08:25] in receive_packet_chat_msg_req_100() [/usr/src/freeciv/common/packets_gen.c::9572]: packet_chat_msg_req_100: got info about ()
5: [T000 - 2020/07/18 20:08:25] in receive_packet_chat_msg_req_100() [/usr/src/freeciv/common/packets_gen.c::9588]: got field 'message'
5: [T000 - 2020/07/18 20:08:25] in start_processing_request() [/usr/src/freeciv/server/sernet.c::1357]: start processing packet 285 from connection 1
5: [T000 - 2020/07/18 20:08:25] in send_packet_processing_started_100() [/usr/src/freeciv/common/packets_gen.c::1105]: packet_processing_started_100: sending info about ()
5: [T000 - 2020/07/18 20:08:25] in send_packet_processing_started_100() [/usr/src/freeciv/common/packets_gen.c::1106]: Json out: {"pid":0}
5: [T000 - 2020/07/18 20:08:25] in add_connection_data() [/usr/src/freeciv/common/networking/connection.c::271]: add 12 bytes to 0 (space =40960)
5: [T000 - 2020/07/18 20:08:25] in write_socket_data() [/usr/src/freeciv/common/networking/connection.c::201]: trying to write 12 limit=0
5: [T000 - 2020/07/18 20:08:25] in send_packet_chat_msg_100() [/usr/src/freeciv/common/packets_gen.c::9046]: packet_chat_msg_100: sending info about ()
5: [T000 - 2020/07/18 20:08:25] in send_packet_chat_msg_100() [/usr/src/freeciv/common/packets_gen.c::9105]: field 'message' has changed
5: [T000 - 2020/07/18 20:08:25] in send_packet_chat_msg_100() [/usr/src/freeciv/common/packets_gen.c::9176]: Json out: {"pid":25,"fields":[1],"message":"christian: 'set mapsize'"}


Related issues

Blocked by Freeciv - Bug #745593: JSON protocol: delta doesn't workClosed

<a title="Actions" class="icon-only icon-actions js-contextmenu" href="#">Actions</a>

History

#1 Updated by christian montanari about 2 years ago

after some debugging...

dio_get_*estring*_json() unescapes the string "%2Fset%20mapsize%20%22FULLSIZE%22" into ...

(gdb) p dest
$2 = 0x7ffc3343c670 "/set mapsize \"FULLSIZE\""

which is still escaped !

yet send_packet_chat_msg_100() field 'message' has changed" uses dio_put_*string*_json()
but with
(gdb) p value
$3 = 0x7ffc33439710 "christian: 'set mapsize'"

wrong already...
whereas in...

bool server_handle_packet() {
...
case PACKET_CHAT_MSG_REQ:
handle_chat_msg_req(pconn,
((const struct packet_chat_msg_req *)packet)->message);
return TRUE;
...

p ((const struct packet_chat_msg_req *)packet)->message
$4 = "/set mapsize \"FULLSIZE\"", '\000' <repeats 1512 times>

is still ok.

but in ....

344 (void) handle_stdin_input(pconn, real_message);
real_message = $5 = "/set mapsize \000FULLSIZE\"\000'\367C`G\177\000\000\000\024\000\000\000\000\000\000`\003N`G\177\000\000╚ĚC3\374\177\000\000\210\n\324`G\177\000\000\260.\323`G\177\000\000`\003N`G\177\000\000\242\b\000\000\004\000\000\000\000V\324`\255\000\000\000\300\003N`G\177\000\000\b\002N`G\177\000\000D\270C3\374\177\000\000\060\000\000\000\060\000\000\000\270\314C3\374\177\000\000\340\313C3\374\177\000\000got packet type=(PACKET_CHAT_MSG_REQ) len=26 from christi"...

this is not correct anymore...
so then the problem should be seen in ...

void handle_chat_msg_req(...) {
....
/* This loop to prevent players from sending multiple lines which can * be abused /
for (cp = real_message; *cp != '\0'; cp++) {
if (*cp '\n' || *cp '\r' || *cp '<' || *cp '>' || *cp '"' || **cp '\''
) {
^^^^^^^

*cp = '\0';
break;
}
}

which crops at '\', it should it accept "\"" ... ?

#2 Updated by christian montanari about 2 years ago

it seems to be a feature of lower versions, and must have a justification.

however * cp '"' needs to be ignored when JSON on?

#3 Updated by Marko Lindqvist almost 2 years ago

Can you give instructions to reproduce the problem. I don't see such problems with my json-enabled build.

#4 Updated by christian montanari almost 2 years ago

aye, aye...

(need all patches as a prerequisite... https://www.hostedredmine.com/issues/745593)

it might be a sordid issue with external json library (I am using debian distro), globing (or not) strings or a problem induced by the later patch ?!

anyway the break limit check-up is weak... am not sure but a max length and some intermediary chunk "end-line" sanity code should be enough?

#5 Updated by christian montanari almost 2 years ago

ah, I forgot, you try to stop shell commands to be inserted ! tricky. in fact one would put a many states parser on those messages?
I just got to the dirtiest fix.

#6 Updated by Marko Lindqvist 5 months ago

  • Blocked by Bug #745593: JSON protocol: delta doesn't work added

#7 Updated by Marko Lindqvist 5 months ago

Still haven't been able to reproduce, neither with delta disabled or enabled (with that #745593 patch)

Also available in: Atom PDF