Method whitelisting

[JensAyton]

The legacy script engine, AI system and shader uniform bindings are all implemented by letting plists call Objective-C methods directly. This is very convenient to implement, but has two major problems:

• It’s insecure. Although not every Objective-C method can be called through this mechanism, it’s impossible to guarantee that nothing dangerous can be done. It’s definitely possible to crash the game by calling unexpected methods.
• It paves the way for bugs. Because there’s no clear distinction between script/AI methods and private methods, any change in Oolite can potentially cause scripts or AIs to stop working. I know of several cases where this has happened already. There are probably several we haven’t noticed.

I plan to fix this using a white list. This is a list – or rather, several lists – of permitted methods; you can see the current lists here.

Whitelisting is now implemented for AIs in the trunk. Initially, it just prints a warning for unknown methods, in order to help find all the methods actually used by AIs. The warning will look like this in 1.73:

Code: Select all

[ai.unpermittedMethod] Handler "UPDATE" for state "GLOBAL" in AI "badAI.plist" uses "naughtyMethod:", which is not a permitted AI method. In a future version of Oolite, this method will be removed from the handler. If you believe the handler should be a permitted method, please report it to <e-mail address redacted>.

Posting reports to this thread is an acceptable alternative to mailing a report. From 1.74, bad selectors will be ignored in accordance with the threat.

I intend to implement similar systems for scripts and uniform bindings before 1.73. Scripts are a bit tricky because of the various loose script conditions found in plists, but it’s not an insurmountable problem. Shaders are easier.

Whitelist violations will probably also be reported by the OXP verifier in 1.73.

The whitelist processing has some fringe benefits: first, it allows simple renaming of methods with backwards compatibility. (Previously, we'd have to add a wrapper method, which added bloat.) See ai_method_aliases at the bottom of whitelist.plist. Second, it makes it easy to convert AIs to a slightly more efficient internal representation.

[Commander McLane]

Wow! That's quite an interesting list. There are methods on it I never have heard of, and I have scrutinized all available documentation several times. I feel almost challenged to write a groundbreaking legacy-scripted OXP (no, not really, don't be afraid).

[Lestradae]

Very useful summary ...

[JensAyton]

Wow! That's quite an interesting list. There are methods on it I never have heard of, and I have scrutinized all available documentation several times. I feel almost challenged to write a groundbreaking legacy-scripted OXP (no, not really, don't be afraid).

While I see your perspective, I’m personally more interested in knowing what’s not on the list.

I should also point out that at least one method there (performDocking) is known not to do anything, but is nevertheless called by one of the standard AIs. Also, setTargetToNearestStation is quasi-new in 1.73 (it’s just setTargetToStation renamed now that we have a convenient renaming mechanism).

Once proper lists of methods are established, they will provide a good basis for a) better documentation and b) my back-burner legacy-to-JS converter.

[Eric Walch]

In Hoopy Casino Giles used setGuiToStatusScreen when he didn't want to return to a mission screen after ending. Currently the systems put you always in this screen after ending a mission screen. Maybe not in old oolite versions and it had to be there.
Anyhow, I knew I had seen this command somewhere in the past and we definitely don't need it now. But maybe it shouldn't break old code. If this command is just skipped in new code there will be no problem.

I myself used often fireMissile. Not in released stuff, but it is a very useful command when testing missiles and missile defence of other ships. Probably this one should go on the white list. For a real AI fightOrFleeHostiles will often be a better way to fire a missile with AI script.

replaceVariablesInString: belongs not on the list. Its between the other commands because it is called by them. I don't think it will work by itself as it returns the result in a way legacy can't use it.

setUpEscorts is also used in several scripts. It was never intended as AI command. Its a bad command for AI because it always adds a new set of escorts, even when there already were some.
But for a mission ship it is the only way to ensure it will have escorts.

[Thargoid]

I myself used often fireMissile. Not in released stuff, but it is a very useful command when testing missiles and missile defence of other ships. Probably this one should go on the white list. For a real AI fightOrFleeHostiles will often be a better way to fire a missile with AI script.

I use that one in Second Wave to get the Thargon launches to occur earlier and make the Thargoid ships react more like "Elite" ones rather than having Thargon launching being a last-minute thing and weakening the whole encounter.

So I would second that one for the whitelist.

[JensAyton]

setGuiToStatusScreen

OK, remapped to new doNothing.

fireMissile

Me too!

Oh, all right then.

replaceVariablesInString:

Good catch.

setUpEscorts

Whitelisted, and moved repeated-setup check into method.

Also, I’ve implemented AI validation in the OXP verifier. As a result of this, I noticed that shuttleAI.plist, which is never used, called two methods that don’t exist at all: performLanding and performFlyToPlanet. If anyone’s using either of those commands, or shuttleAI.plist, their OXP is broken.

[JensAyton]

Whitelisting is now implemented for uniform shader bindings.

[Commander McLane]

One more for the whitelist:

Code: Select all

spawn:

isn't mentioned yet. Should be on the ai_and_action_methods side of things, I think.

[JensAyton]

Done.

Whitelisting for scripts is mostly done, but there are some issues to iron out. I should be able to finish it this weekend.

As a side effect, the whitelisting system will transform code into a new internal format. This is primarily a security feature – the updated script engine won’t be able to run “unsanitized” code – but will also make scripts run a bit faster.

[Svengali]

throwSparks and abortAllDockings for AIs would also be nice.
I'm using both things in the Localhero2 and I haven't seen any problems.

[JensAyton]

throwSparks shouldn’t be called directly, but rather by setting the throw_sparks flag to true, which you can’t do from a script. On the other hand, this could be done with a script method called throwSparks, so it will be. :-)

abortAllDockings doesn’t look like a problem either.

[Svengali]

Muchas gracias, Ahruman :-)