commits/15790

Change 15790

tjuricek
tjuricek committed this change 10 years ago into //guest/perforce_software/helix-web-services/main/source
Request Review
Download .zip
Do not allow rsh: P4PORT values to be set
Add Job
3 edited 0 added 0 deleted
Tip: Use n and p to cycle through the changes.
gnicol (gnicol)
if hws_settings.P4PORT
+ if hws_settings.P4PORT.start_with?('rsh:')
+ fail P4Error.new(0, 3, 'Do not use rsh: P4PORT values')
+ end
gnicol (on p4_util.rb, line 32) commented 10 years ago

Did you consider special characters?
E.g. a port of %00rsh: or space rsh: or similar?
Reply
gnicol (gnicol)
if hws_settings.P4PORT
+ if hws_settings.P4PORT.start_with?('rsh:')
+ fail P4Error.new(0, 3, 'Do not use rsh: P4PORT values')
+ end
gnicol (on p4_util.rb, line 32) commented 10 years ago

Or RSH or rSh etc.
Reply
tjuricek
if hws_settings.P4PORT
+ if hws_settings.P4PORT.start_with?('rsh:')
+ fail P4Error.new(0, 3, 'Do not use rsh: P4PORT values')
+ end
tjuricek (on p4_util.rb, line 32) commented 10 years ago

What special characters are interpreted by the underlying system? I'm not looking to block anything but keep the p4api from exec-ing random shit.

I'll make it case insensitive though, that'd be good
Reply
gnicol (gnicol)
if hws_settings.P4PORT
+ if hws_settings.P4PORT.start_with?('rsh:')
+ fail P4Error.new(0, 3, 'Do not use rsh: P4PORT values')
+ end
gnicol (on p4_util.rb, line 32) commented 10 years ago

No idea. I wouldn't be shocked if whitespace was ignored by p4d (haven't tested don't know) though.

Perhaps there is a more secure way to just inform p4-api you don't want to permit rsh ports?
Or to ask p4-api to parse the report and tell you its type before using it?
Reply
gnicol (gnicol)
if hws_settings.P4PORT
+ if hws_settings.P4PORT.start_with?('rsh:')
+ fail P4Error.new(0, 3, 'Do not use rsh: P4PORT values')
+ end
gnicol (on p4_util.rb, line 32) commented 10 years ago

Quick glance it looks like 'jsh:' is also a thing and will allow arbitrary command execution.

Blocking rsh: and jsh: when it lands just anywhere isn't a good idea.

It would block blundersh:1666 for example which is a perfectly viable port.

Given your architecture, I'd strongly suggest looking into having p4-api parse the port then asking it to tell you if its this sorta style.
Or adding a flag to the api (if there isn't one) to get it to block execing for port.
Reply
tjuricek
if hws_settings.P4PORT
+ if hws_settings.P4PORT.start_with?('rsh:')
+ fail P4Error.new(0, 3, 'Do not use rsh: P4PORT values')
+ end
tjuricek (on p4_util.rb, line 32) commented 10 years ago

Hm, appears I have more work to do.

I made another commit related to this topic. Disregard that. I'm gonna have to sift through the P4API for a bit to figure out what to do.
Reply
gnicol (gnicol)
if hws_settings.P4PORT
+ if hws_settings.P4PORT.start_with?('rsh:')
+ fail P4Error.new(0, 3, 'Do not use rsh: P4PORT values')
+ end
gnicol (on p4_util.rb, line 32) commented 10 years ago

Cool thanks for taking the feedback!
Reply